Closed
Bug 393770
Opened 17 years ago
Closed 17 years ago
Crash [@ nsCachedStyleData::GetStyleData] from [@ nsQuoteNode::Text]
Categories
(Core :: General, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: pvnick, Assigned: MatsPalmgren_bugz)
References
Details
(Keywords: crash, verified1.8.0.14, verified1.8.1.8, Whiteboard: [sg:critical?] wfm trunk. using freed frame)
Crash Data
Attachments
(7 files, 1 obsolete file)
3.22 KB,
text/plain
|
Details | |
11.07 KB,
text/html
|
Details | |
8.97 KB,
text/plain
|
Details | |
1.18 KB,
patch
|
Details | Diff | Splinter Review | |
5.43 KB,
patch
|
Details | Diff | Splinter Review | |
17.90 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
19.01 KB,
patch
|
bzbarsky
:
superreview+
dveditz
:
approval1.8.1.8+
dveditz
:
approval1.8.0.14+
|
Details | Diff | Splinter Review |
Firefox version: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070812 BonEcho/2.0.0.6 Details: eax=02717b70 ebx=023ac510 ecx=0271c260 edx=0270ba54 esi=0012fb70 edi=02717b28 eip=02717a75 esp=0012fb3c ebp=0012fb5c iopl=0 nv up ei pl nz na po cy cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00000203 02717a75 224402a8 and al,byte ptr [edx+eax-58h] ds:0023:04e2356c=?? Disassembly: gklayout!nsIFrame::GetStateBits+0xa: 0197f53a 8b4024 mov eax,dword ptr [eax+24h] 0197f53d 8be5 mov esp,ebp 0197f53f 5d pop ebp 0197f540 c3 ret 0197f541 cc int 3 0197f542 cc int 3 0197f543 cc int 3 0197f544 cc int 3 Stack trace (using release build - debug build didnt crash): 0x2717a75 xpcom_core!nsQueryInterface::operator() firefox!Java_netscape_javascript_JSObject_initClass firefox!nsFontList::Release firefox!jpeg_mem_init firefox!nsPrintOptions::GetNewPrintSettings firefox!nsPrintOptions::GetNewPrintSettings firefox!nsPrintOptions::GetNewPrintSettings firefox!nsPrintOptions::GetNewPrintSettings firefox!nsPrintOptions::GetNewPrintSettings xpcom_core!PL_HandleEvent xpcom_core!PL_ProcessPendingEvents xpcom_core!PL_IsQueueNative This crashes all over the place, and definitely looks exploitable, IMO. Also, I wasn't able to reduce the testcase any further - maybe someone else will have more luck.
Flags: blocking1.8.1.7?
Flags: blocking1.8.0.13?
Reporter | ||
Comment 1•17 years ago
|
||
[STACKHASH:AD5295AC3574BB6B0A3CB7DBA1455E8]
Comment 2•17 years ago
|
||
Paul, could you create a testcase that crashes online, directly? You have external references to localhost in your source.
Reporter | ||
Comment 3•17 years ago
|
||
This should work. If anyone can further reduce this testcase, they should do so since it exposes the existence and partial workings of the fuzzer scripts.
Comment 4•17 years ago
|
||
Thanks. This somehow was fixed on trunk between 2006-05-17 and 2006-05-17: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-05-17+07&maxdate=2006-05-18+09&cvsroot=%2Fcvsroot The only bug I see in that range that might somehow be involved is bug 329122.
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → mats.palmgren
Keywords: crash
OS: Windows XP → All
Hardware: PC → All
Summary: Crash [@xpcom_core!nsQueryInterface::operator] → Crash [@ nsCachedStyleData::GetStyleData] from [@ nsQuoteNode::Text]
Whiteboard: [sg:critical?] using freed frame
Assignee | ||
Comment 5•17 years ago
|
||
Assignee | ||
Comment 6•17 years ago
|
||
Frames can be destroyed while nsQuoteList::RecalcAll is iterating the list. nsCSSFrameConstructor::NotifyDestroyingFrame removes the corresponding quote list node for the destroyed frame...
Assignee | ||
Comment 7•17 years ago
|
||
Assignee | ||
Comment 8•17 years ago
|
||
This avoids the crash. Looking at nsCounterList::RecalcAll(), I think the same thing could happen there also: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCounterManager.cpp&rev=3.1.12.1&root=/cvsroot&mark=184#163
Comment 9•17 years ago
|
||
So on trunk this is OK because no one sees these events, right? Can we make that change on branch too? Just not propagate mutation events out of anonymous content?
Comment 10•17 years ago
|
||
Mutation events shouldn't propagate from native anonymous content on branch either, hmm, unless the nsIContent is a nsGenericDOMDataNode. Is that the case here? Should the same thing be done for nsGenericDOMDataNode as what was done for nsGenericElement in Bug 382681. Problem is that on branch nsGenericDOMDataNode doesn't implement SetNativeAnonymous() http://lxr.mozilla.org/mozilla1.8/source/content/base/src/nsGenericDOMDataNode.cpp#742
Comment 11•17 years ago
|
||
Yes, the anon content is a nsTextNode here. Can we use GetBindingParent() == this as the native anonymous test on branch?
Comment 12•17 years ago
|
||
Unfortunately no, because bug 329122 is trunk only :(
Comment 13•17 years ago
|
||
Ah, right. :(
Assignee | ||
Comment 14•17 years ago
|
||
Assignee | ||
Comment 15•17 years ago
|
||
(In reply to comment #9) > So on trunk this is OK because no one sees these events, right? Yes, I disabled the code that prevents these events on trunk and got the exact same crash on trunk. > Can we make that change on branch too? I (naively) implemented nsGenericDOMDataNode::SetNativeAnonymous() by setting a flag on nsGenericDOMDataNode and used that to avoid calling HandleDOMEvent() in nsGenericDOMDataNode::HandleDOMEvent(). This fixes the bug, but we'd need to find a place for the extra bit... (the two bits we have in mParentPtrBits are already in use AFAICT)
Comment 16•17 years ago
|
||
Could we steal a bit from the length in nsTextFragment?
Even better would be to use a bit from mNodeInfoManager
Updated•17 years ago
|
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.7?
Flags: blocking1.8.1.7+
Flags: blocking1.8.0.14?
Flags: blocking1.8.0.13?
Assignee | ||
Comment 19•17 years ago
|
||
Correction for SetNativeAnonymous().
Attachment #278694 -
Attachment is obsolete: true
Attachment #278813 -
Flags: review?(jonas)
Attachment #278694 -
Flags: review?(jonas)
Yes, though I would rename mNodeInfoManager to mNodeInfoManagerPtrBits to avoid someone using it wrongly accidentally.
Comment on attachment 278813 [details] [diff] [review] wip4 (branch 1.8) r=me if you rename mNodeInfoManager to mNodeInfoManagerPtrBits and add a define for the 0x1 flag used both in Get/SetNativeAnonymous and in GetNodeInfoManager
Attachment #278813 -
Flags: review?(jonas) → review+
Comment 22•17 years ago
|
||
since vendors sometimes backport the patches in the bugs rather than the actual checkins (which is very naughty of them), please request branch approval on a patch that incorporates Jonas's comment 21
Whiteboard: [sg:critical?] using freed frame → [sg:critical?] wfm trunk, need 1.8 approval request. using freed frame
Comment 23•17 years ago
|
||
Mats: is this going to make the 1.8.1.8 code freeze (Oct 3)?
Assignee | ||
Comment 24•17 years ago
|
||
Addressing Jonas' review comment.
Attachment #283290 -
Flags: approval1.8.1.8?
Attachment #283290 -
Flags: approval1.8.0.14?
Assignee | ||
Updated•17 years ago
|
Whiteboard: [sg:critical?] wfm trunk, need 1.8 approval request. using freed frame → [sg:critical?] wfm trunk. using freed frame
Comment 25•17 years ago
|
||
Comment on attachment 283290 [details] [diff] [review] wip5 (branch 1.8) approved for 1.8.1.8 and 1.8.0.14, a=dveditz
Attachment #283290 -
Flags: approval1.8.1.8?
Attachment #283290 -
Flags: approval1.8.1.8+
Attachment #283290 -
Flags: approval1.8.0.14?
Attachment #283290 -
Flags: approval1.8.0.14+
Assignee | ||
Comment 26•17 years ago
|
||
Comment on attachment 283290 [details] [diff] [review] wip5 (branch 1.8) guess I need an sr here before checkin...
Attachment #283290 -
Flags: superreview?(bzbarsky)
Comment 27•17 years ago
|
||
Comment on attachment 283290 [details] [diff] [review] wip5 (branch 1.8) sr=bzbarsky
Attachment #283290 -
Flags: superreview?(bzbarsky) → superreview+
Assignee | ||
Comment 28•17 years ago
|
||
MOZILLA_1_8_BRANCH mozilla/content/base/src/nsCommentNode.cpp 3.72.8.4 mozilla/content/base/src/nsDOMDocumentType.cpp 1.27.20.5 mozilla/content/base/src/nsGenericDOMDataNode.cpp 3.156.2.9 mozilla/content/base/src/nsGenericDOMDataNode.h 3.98.8.9 mozilla/content/base/src/nsTextNode.cpp 3.49.8.4 mozilla/content/xml/content/src/nsXMLCDATASection.cpp 1.39.4.4 mozilla/content/xml/content/src/nsXMLProcessingInstruction.cpp 1.60.8.5 MOZILLA_1_8_0_BRANCH mozilla/content/base/src/nsCommentNode.cpp 3.72.14.2 mozilla/content/base/src/nsDOMDocumentType.cpp 1.27.26.2 mozilla/content/base/src/nsGenericDOMDataNode.cpp 3.156.8.4 mozilla/content/base/src/nsGenericDOMDataNode.h 3.98.14.3 mozilla/content/base/src/nsTextNode.cpp 3.49.14.2 mozilla/content/xml/content/src/nsXMLCDATASection.cpp 1.39.10.2 mozilla/content/xml/content/src/nsXMLProcessingInstruction.cpp 1.60.14.2 mozilla/content/xml/content/src/nsXMLStylesheetPI.cpp 1.12.14.2 -> FIXED
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: blocking1.8.0.14? → in-testsuite?
Keywords: fixed1.8.0.14,
fixed1.8.1.8
Resolution: --- → FIXED
Comment 29•17 years ago
|
||
Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.8pre) Gecko/20071004 BonEcho/2.0.0.8pre I can see the crash in Firefox2.0.0.7.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1.8 → verified1.8.1.8
Updated•17 years ago
|
Group: security
Comment 30•17 years ago
|
||
This broke Find code on branch. See bug 403090.
Comment 31•17 years ago
|
||
Verified in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre.
Keywords: fixed1.8.0.14 → verified1.8.0.14
Updated•13 years ago
|
Crash Signature: [@ nsCachedStyleData::GetStyleData]
[@ nsQuoteNode::Text]
You need to log in
before you can comment on or make changes to this bug.
Description
•