330563 - Crash with evil xbl testcase, using alert in constructor

Status: VERIFIED FIXED

(Core :: XBL, defect)

Description

[Martijn Wargers (dead)]

See upcoming testcase, which crash Mozilla after clicking for the second time on the alert.
Also crashes Mozilla1.7, so no (recent) regression.

Talkback ID: TB16399392W
0x00000000
nsCSSFrameConstructor::WipeContainingBlock   nsCSSFrameConstructor::ContentInserted  

Marking security sensitive, because the first line of the stack looks funny.
Please unmark the security flag, if this is not a security issue.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

The testcase crashes for me after clicking for the second time on the alert.

Comment 2

[Martijn Wargers (dead)]

Ok, online it's not crashing directly. But after the first time clicking back and then clicking another 2 times on the alert makes Mozilla crash.
Or you could test the testcase locally.

Comment 3

[Martijn Wargers (dead)]

Attachment: Backtrace from debug build

#0  0xdddddddd in ?? ()
#1  0x055ccc0b in nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorS
tate&, nsIFrame*, nsIFrame*, nsIFrame*) (this=0xe7abf48, aState=@0x22f1e8,
    aContainingBlock=0xe82b82c, aFrame=0xe82b82c, aFrameList=0x0)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:13220
#2  0x055c5049 in nsCSSFrameConstructor::ContentInserted(nsIContent*, nsIContent
*, int, nsILayoutHistoryState*, int) (this=0xe7abf48, aContainer=0x3c10e50,
    aChild=0xe7ff698, aIndexInContainer=2, aFrameState=0x0,
    aInReinsertContent=0)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:9659
#3  0x0561327c in PresShell::ContentInserted(nsIDocument*, nsIContent*, nsIConte
nt*, int) (this=0xe72fb38, aDocument=0xe80ddc0, aContainer=0x3c10e50,
    aChild=0xe7ff698, aIndexInContainer=2)
    at c:/mozilla/mozilla/layout/base/nsPresShell.cpp:5211
etc..

Comment 4

[Daniel Veditz [:dveditz]]

In a 1.5.0.2-ish debug build clicking alert twice still did it. The crash was below WipeContainingBlock, but similarly used deleted-looking objects ('this' was 0xddddddf9)

>nsCachedStyleData::GetStyleData
 nsStyleContext::GetStyleData
 nsIFrame::GetStyleData
 nsIFrame::GetStyleDisplay
 nsCSSFrameConstructor::WipeContainingBlock
 ...etc...
    

Comment 5

[Boris Zbarsky [:bzbarsky]]

Probably a manifestation of bug 267833, more or less.

Comment 6

[Daniel Veditz [:dveditz]]

Critical security bugs must have owners. If you can't work on this bug please help us find another active owner for it.

Comment 7

[chris hofmann]

jonas, had a chance to look at this enough to figure out if its a bug you can take, or need to pass to others?

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I haven't had a chance to look thoroughly yet, but like bz says, I strongly suspect this will get magically fixed when bug 267833 is fixed.

Comment 9

[Martijn Wargers (dead)]

This seems to work fine now in current trunk build, but I have to test the testcase locally, because of bug 373536.
This one is fixed now, fixed by bug 267833.

Comment 10

[chris hofmann]

doesn't look fixed on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3) Gecko/2007030919 Firefox/2.0.0.3 -  should we be considering getting the patches

I still crash on the test case using that build.  should we be considering the patch for 267833 on the 1.8 branch?

Comment 11

[chris hofmann]

stack for my crash on 2.0.0.3 looks like

Access violation
Source File, Line No.	c:/builds/tinderbox/Fx-Mozilla1.8-release/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13566
Stack Trace 	
nsCSSFrameConstructor::WipeContainingBlock  [mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13566]
nsCSSFrameConstructor::ContentInserted  [mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9575]
PresShell::ContentInserted  [mozilla/layout/base/nsPresShell.cpp, line 5538]
nsXBLStreamListener::Load  [mozilla/content/xbl/src/nsXBLService.cpp, line 420]
DispatchToInterface  [mozilla/content/events/src/nsEventListenerManager.cpp, line 144]
nsEventListenerManager::HandleEvent  [mozilla/content/events/src/nsEventListenerManager.cpp, line 1752]
nsDocument::HandleDOMEvent  [mozilla/content/base/src/nsDocument.cpp, line 4075]
nsXMLDocument::EndLoad  [mozilla/content/xml/document/src/nsXMLDocument.cpp, line 657]
nsXMLContentSink::DidBuildModel  [mozilla/content/xml/document/src/nsXMLContentSink.cpp, line 353]
nsExpatDriver::DidBuildModel  [mozilla/parser/htmlparser/src/nsExpatDriver.cpp, line 1227]

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Bug 267833 hasn't landed on branch yet (and lets discuss over there if it should or not).

Comment 13

[Daniel Veditz [:dveditz]]

Marking "wanted" to indicate this at least happens on the branch.

Comment 14

[Daniel Veditz [:dveditz]]

Blocking for now, will talk to Jonas about bug 267833 (and its regressions) for the branch when he's back, or whether there might be a smaller crash band-aide.

Comment 15

[Daniel Veditz [:dveditz]]

Jonas: any thoughts re: comment 14? bug 267833 itself appears to still have open trunk regressions.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't think it's worth it trying to patch this one instead of bug 267833 on the branch. We could spend forever patching symptoms while leaving the real problem there.

Comment 17

[Daniel Veditz [:dveditz]]

Not going to land 267833 on the branches with open regressions, so not making this release.

Comment 18

[Daniel Veditz [:dveditz]]

bug 267833 landed on the 1.8 branch and fixed this.

Comment 19

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.8) Gecko/20071004 Firefox/2.0.0.8: Firefox 2.0.0.8 ID:2007100415.    No further crashes.