Closed Bug 296449 Opened 19 years ago Closed 19 years ago

Error in string handling within <keygen>

Categories

(Core :: Security: PSM, defect)

Product:
Core
Component:
Security: PSM
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: agoralski, Assigned: pkwarren)

Details

Attachments

(1 file, 1 obsolete file)

Patch for checkin
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.
I can reproduce the crash.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch Patch v1 (obsolete) — — Splinter Review 
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.
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 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+
Is anyone else able to review this patch?
Comment on attachment 185215 [details] [diff] [review]
Patch v1

Thanks for the patch

r=kaie
Attachment #185215 - Flags: review?(kai.engert) → review+
Patch addressing dveditz's comments. Requesting approval for current releases.
Assignee: kaie.bugs → pkwarren
Attachment #185215 - Attachment is obsolete: true
Status: NEW → ASSIGNED
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 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-
Attachment #189430 - Flags: approval1.8b4? → approval1.8b4+
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: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: