Closed Bug 468538 Opened 12 years ago Closed 12 years ago

Crash [@ nsParser::ParseFragment] setting innerHTML in mixed-content document


(Core :: DOM: HTML Parser, defect)

Not set





(Reporter: jruderman, Assigned: bent.mozilla)



(6 keywords, Whiteboard: [sg:dos] null deref)

Crash Data


(3 files, 1 obsolete file)

ML Parsing Error: prefix not bound to a namespace
Line Number 1, Column 1:
<xul:box><div xmlns="">

###!!! ASSERTION: ParseFragment requires a fragment content sink: 'fragSink', file /Users/jruderman/central/parser/htmlparser/src/nsParser.cpp, line 2073

###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../../dist/include/xpcom/nsCOMPtr.h, line 796

Regression from bug 460437, perhaps?
Most likely.
Assignee: nobody → bent.mozilla
Flags: blocking1.9.1?
Blocks: 460437
Attached patch Patch, v1 (obsolete) — Splinter Review
So... This fixes the crash. But it's kinda ugly.

Basically Parse() is failing but returning NS_OK. I think that is the real problem, but I can't imagine how much other code might depend on that currently. So this may be the simplest thing to do for now. mrbkap, what do you think?
Attachment #352020 - Flags: superreview?(mrbkap)
Attachment #352020 - Flags: review?(mrbkap)
Comment on attachment 352020 [details] [diff] [review]
Patch, v1

I'd rather not do this if possible. Calling into the parser when no sink is null is a dangerous proposition at best. Can we, instead detect that the sink became null after the call to parse (assert that this is the XML mode) and bail early if so?
Flags: blocking1.9.1? → blocking1.9.1+
Attached patch Patch, v1.1Splinter Review
Attachment #352020 - Attachment is obsolete: true
Attachment #353134 - Flags: superreview?(mrbkap)
Attachment #353134 - Flags: review?(mrbkap)
Attachment #352020 - Flags: superreview?(mrbkap)
Attachment #352020 - Flags: review?(mrbkap)
Attachment #353134 - Flags: superreview?(mrbkap)
Attachment #353134 - Flags: superreview+
Attachment #353134 - Flags: review?(mrbkap)
Attachment #353134 - Flags: review+
Comment on attachment 353134 [details] [diff] [review]
Patch, v1.1

Yeah, let's try this.
Pushed changeset 324151b8a4b9 to mozilla-central.
Closed: 12 years ago
Resolution: --- → FIXED
Pushed changeset 445c44c3c804 to mozilla-1.9.1
Keywords: fixed1.9.1
Flags: in-testsuite+
Verified on trunk and 1.9.1 with:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090131 Minefield/3.2a1pre ID:20090131034630

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090201 Minefield/3.2a1pre ID:20090201020604

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090201 Shiretoko/3.1b3pre ID:20090201033606

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090201 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090201020520
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.2a1
1.9.0 !exploitable report:
PROBABLY_EXPLOITABLE: Probably Exploitable - Data from Faulting Address controls Code Flow starting at gkparser!nsParser::ParseFragment
Flags: blocking1.9.0.11?
(responding as "private" since comment 9 is private).

This isn't exploitable -- we're calling a virtual function through a guaranteed null pointer. This is a false positive like the one that dveditz pointed out on the s-g list.
We've taken bug 460437 in the release, we should probably take this regression fix before we ship a release with the problem.
Flags: wanted1.9.0.x+
Whiteboard: [sg:dos] null deref
Keywords: regression
Flags: blocking1.9.0.11? → blocking1.9.0.11+
Comment on attachment 376091 [details] [diff] [review]
Patch for 1.9.0 branch

Approved for a=ss for release-drivers
Attachment #376091 - Flags: approval1.9.0.11? → approval1.9.0.11+
Pushed rev 3.412 of nsParser.cpp to cvs.
Verified for with my debug build: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv: Gecko/2009051111 GranParadiso/3.0.11pre. No crash.
Doesn't affect the 1.8.1 branch
Flags: wanted1.8.1.x-
Crash Signature: [@ nsParser::ParseFragment]
You need to log in before you can comment on or make changes to this bug.