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

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: guninski, Assigned: jst)

Tracking

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

Trunk
x86
Linux
Points:
---
Dependency tree / graph
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.
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+
jst to have a look
Assignee: nobody → jst
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.
Attachment #162786 - Flags: superreview?(dbaron)
Attachment #162786 - Flags: review?(darin)
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+
Fixed on the branches. Not a problem on the trunk.
Status: NEW → RESOLVED
Closed: 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.
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.