Closed
Bug 296449
Opened 20 years ago
Closed 20 years ago
Error in string handling within <keygen>
Categories
(Core :: Security: PSM, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: agoralski, Assigned: pkwarren)
Details
Attachments
(1 file, 1 obsolete file)
|
763 bytes,
patch
|
pkwarren
:
review+
pkwarren
:
superreview+
chase
:
approval-aviary1.0.6-
chase
:
approval1.7.10-
benjamin
:
approval1.8b4+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl-PL; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl-PL; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
When you put <keygen> tag into the form then it's transformed into a
simple <SELECT> tag with two <OPTIONS> - 2048 (High grade) and 1024
(Medium grade). But when looking in DOM inspector i've noticed that it
is possible to manipulate these objects (options, to be precise)
trough JavaScript.
After manipulation, KeygenHandler (source:
security/manager/ssl/src/nsKeygenHandler.cpp) is getting a different
string (other size) than it's expecting, causing a crash. Since
KeygenHandler is depending on string values rather than option ID's,
maybe it's possible to create a buffer overflow, and inject external
code? I'm not a CPP guru, so i don't know how to exploit this bug,
please check that.
Reproducible: Always
Steps to Reproduce:
Here's the code causing the crash (in the code i've changed the first
text value of Keygen options):
<html>
<body onload="testit()">
<script language="javascript">
function testit() {
var s;
s = document.getElementsByTagName("option");
s[0].text='00000000000000000000000000000000000';
document.crashform.submit();
}
</script>
<form name="crashform">
<keygen name="ffkey">
</form>
</body>
</html>
Actual Results:
Mozilla Firefox 1.04 under Windows XP: crash
Mozilla Firefox 1.03 under Windows XP: crash
Mozilla Firefox 1.02 under Linux: crash
Mozilla Suite 1.6 under Windows 2000: crash
Deer Park Alpha 1 under Windows 2000: crash
Expected Results:
Display error stopping the key generation process.
|
Assignee | |
Comment 1•20 years ago
|
||
I can reproduce the crash.
Status: UNCONFIRMED → NEW
Ever confirmed: true
|
|
Assignee | |
Comment 2•20 years ago
|
||
Fixes the crash. The code was looking for a match of one of the two strings,
and wasn't correctly exiting the loop when a match was not found.
|
||
Comment 3•20 years ago
|
||
I don't see a security issue here. The code just loops off into space comparing
the input string to chunks of random memory, eventually getting an access
violation trying to read something.
Assignee: nobody → kaie
Group: security
Component: Security → Security: PSM
Product: Firefox → Core
QA Contact: firefox
Version: unspecified → Trunk
|
|
||
Comment 4•20 years ago
|
||
Comment on attachment 185215 [details] [diff] [review]
Patch v1
>- while (choice) {
>+ while (choice && choice->name) {
You don't need to preserve the check for choice, there's really no way for it
to be null. Similar pattern of checking choice for null down in the
ProvideContent method, but that one has the correct check for choice->name in
addition.
sr=dveditz
a=dveditz for the trunk once you've got r=
Attachment #185215 -
Flags: superreview+
Attachment #185215 -
Flags: review?(kai.engert)
Attachment #185215 -
Flags: approval1.8b3+
|
|
Assignee | |
Comment 5•20 years ago
|
||
Is anyone else able to review this patch?
|
||
Comment 6•20 years ago
|
||
Comment on attachment 185215 [details] [diff] [review]
Patch v1
Thanks for the patch
r=kaie
Attachment #185215 -
Flags: review?(kai.engert) → review+
|
|
Assignee | |
Comment 7•20 years ago
|
||
Patch addressing dveditz's comments. Requesting approval for current releases.
Assignee: kaie.bugs → pkwarren
Attachment #185215 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
|
|
Assignee | |
Updated•20 years ago
|
Attachment #189430 -
Flags: superreview+
Attachment #189430 -
Flags: review+
Attachment #189430 -
Flags: approval1.8b4?
Attachment #189430 -
Flags: approval1.7.10?
Attachment #189430 -
Flags: approval-aviary1.0.6?
|
||
Comment 8•20 years ago
|
||
Comment on attachment 189430 [details] [diff] [review]
Patch for checkin
Per 1.0.6 meeting, minusing patch.
Attachment #189430 -
Flags: approval1.7.10?
Attachment #189430 -
Flags: approval1.7.10-
Attachment #189430 -
Flags: approval-aviary1.0.6?
Attachment #189430 -
Flags: approval-aviary1.0.6-
|
||
Updated•20 years ago
|
Attachment #189430 -
Flags: approval1.8b4? → approval1.8b4+
|
|
Assignee | |
Comment 9•20 years ago
|
||
Fixed on trunk.
Checking in nsKeygenHandler.cpp;
/cvsroot/mozilla/security/manager/ssl/src/nsKeygenHandler.cpp,v <--
nsKeygenHandler.cpp
new revision: 1.36; previous revision: 1.35
done
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•