null dereference in nsFormControlHelper::GetType (found by mangler)

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
15 years ago
13 years ago

People

(Reporter: guninski, Assigned: jst)

Tracking

({fixed-aviary1.0, fixed1.7.5})

Trunk
x86
Linux
Points:
---
Bug Flags:
blocking1.7.5 +
blocking-aviary1.0 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; rv:1.7.3) Gecko/20040914 Firefox/0.10
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; rv:1.7.3) Gecko/20040914 Firefox/0.10

found by mangler.cgi.

will attach a testcase.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1083359104 (LWP 13105)]
0x419e9aa0 in nsFormControlHelper::GetType(nsIContent*) (aContent=0x89137c8)
    at nsFormControlHelper.cpp:359
359       return formControl->GetType();
(gdb) p formControl
$1 = {mRawPtr = 0x0}



Reproducible: Always
Steps to Reproduce:
check the testcase
Confirming
Assignee: general → nobody
Blocks: Zalewski
Status: UNCONFIRMED → NEW
Component: Browser-General → Layout
Ever confirmed: true
Flags: blocking1.7.x?
Flags: blocking-aviary1.0?
QA Contact: general → core.layout
why is this a security bug? it's just a crash.

Comment 4

15 years ago
think we want to try for this on the branches before 1.0 ships
Flags: blocking1.7.x?
Flags: blocking1.7.x+
Flags: blocking-aviary1.0?
Flags: blocking-aviary1.0+

Comment 5

15 years ago
jst to have a look
Assignee: nobody → jst
(Assignee)

Comment 8

15 years ago
The underlying reason for this crash is that our parser code permits '\0'
characters in identifiers, but our content creation and frame creation can't
deal, and shouldn't need to, as it's never valid to have '\0' characters in
tag/attribute names.
(Assignee)

Updated

15 years ago
Attachment #162786 - Flags: superreview?(dbaron)
Attachment #162786 - Flags: review?(darin)

Comment 9

15 years ago
Comment on attachment 162786 [details] [diff] [review]
diff -w of the above for review.

r=darin
Attachment #162786 - Flags: review?(darin) → review+
Attachment #162786 - Flags: superreview?(dbaron) → superreview+
Comment on attachment 162786 [details] [diff] [review]
diff -w of the above for review.

a=asa for branches checkin.
Attachment #162786 - Flags: approval1.7.x+
Attachment #162786 - Flags: approval-aviary+
(Assignee)

Comment 11

15 years ago
Fixed on the branches. Not a problem on the trunk.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
> Don't allow embedded null characters in tag names.
(In reply to comment #11)
> Not a problem on the trunk.

that's because bug 264956 fixed it on trunk, I guess; with a different patch of
course, which also has approval, but's not checked in yet.
Group: security
Note that the patch in bug 264956 is far more correct than this one (and covers
more cases, for that matter).  I would much prefer to see this patch backed out
and this one go in.

The wonders of hidden security bugs that people who know the code can't find. 
Gotta love them.
(Assignee)

Comment 14

15 years ago
Done (bug remains fixed, by the patch in bug 264956).
You need to log in before you can comment on or make changes to this bug.