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

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[georgi - hopefully not receiving bugspam]

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

Comment 1

[georgi - hopefully not receiving bugspam]

Attachment: null dereference

Comment 2

[Daniel Veditz [:dveditz]]

Confirming

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

why is this a security bug? it's just a crash.

Comment 4

[chris hofmann]

think we want to try for this on the branches before 1.0 ships

Comment 5

[chris hofmann]

jst to have a look

Comment 6

[Johnny Stenback (:jst)]

Attachment: Don't allow embedded null characters in tag names.

Comment 7

[Johnny Stenback (:jst)]

Attachment: diff -w of the above for review.

Comment 8

[Johnny Stenback (: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.

Comment 9

[Darin Fisher]

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

r=darin

Comment 10

[Asa Dotzler [:asa]]

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

a=asa for branches checkin.

Comment 11

[Johnny Stenback (:jst)]

Fixed on the branches. Not a problem on the trunk.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

> 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.

Comment 13

[Boris Zbarsky [:bzbarsky]]

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.

Comment 14

[Johnny Stenback (:jst)]

Done (bug remains fixed, by the patch in bug 264956).

Comment 15

[Boris Zbarsky [:bzbarsky]]

Thanks, jst!