Crash [@ nsContentUtils::ComparePosition] with binding

VERIFIED FIXED

Status

()

Core
XBL
--
critical
VERIFIED FIXED
10 years ago
7 years ago

People

(Reporter: Martijn Wargers (zombie), Assigned: smaug)

Tracking

(5 keywords)

Trunk
crash, fixed1.9.0.6, regression, testcase, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 -
blocking1.9.0.6 +
wanted1.9.0.x +
wanted1.8.1.x -
wanted1.8.0.x -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] post 1.8-branch, crash signature)

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
Created attachment 344166 [details]
testcase

See testcase, which crashes in current trunk build.

This regressed between 2008-06-22 and 2008-06-23:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-06-22+04%3A00%3A00&enddate=2008-06-23+08%3A00%3A00
I think a regression from bug 344258.

Stacktrace from debug build:
 	gklayout.dll!nsContentUtils::GetContextForEventHandlers(nsINode * aNode=0x09af6478, nsIScriptContext * * aContext=0x00000004)  Line 4354 + 0x3 bytes	C++
>	gklayout.dll!nsGenericElement::GetContextForEventHandlers(nsIScriptContext * * aContext=0x00000004)  Line 346 + 0xd bytes	C++
 	gklayout.dll!nsContentUtils::ComparePosition(nsINode * aNode1=0x0883bac8, nsINode * aNode2=0x09af6478)  Line 1533 + 0xf bytes	C++
 	gklayout.dll!nsContentUtils::PositionIsBefore(nsINode * aNode1=0x0883bac8, nsINode * aNode2=0x09af6478)  Line 280 + 0xd bytes	C++
 	gklayout.dll!nsIdentifierMapEntry::AddIdContent(nsIContent * aContent=0x0883bac8)  Line 459 + 0xd bytes	C++
 	gklayout.dll!nsDocument::UpdateIdTableEntry(nsIContent * aContent=0x0883bac8)  Line 2272	C++
 	gklayout.dll!nsXULDocument::AddElementToDocumentPre(nsIContent * aElement=0x0883bac8)  Line 1625	C++
 	gklayout.dll!nsXULDocument::AddSubtreeToDocument(nsIContent * aElement=0x0883bac8)  Line 1705 + 0x12 bytes	C++
 	gklayout.dll!nsXBLBinding::InstallAnonymousContent(nsIContent * aAnonParent=0x0883ba78, nsIContent * aElement=0x0997dd38)  Line 371	C++
 	gklayout.dll!nsXBLBinding::GenerateAnonymousContent()  Line 663	C++
 	gklayout.dll!nsXBLService::LoadBindings(nsIContent * aContent=0x0997dd38, nsIURI * aURL=0x095f2ee8, nsIPrincipal * aOriginPrincipal=0x087028e8, int aAugmentFlag=0, nsXBLBinding * * aBinding=0x0012eb5c, int * aResolveStyle=0x0012eb30)  Line 631	C++
 	gklayout.dll!nsCSSFrameConstructor::ConstructFrameInternal(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x0997dd38, nsIFrame * aParentFrame=0x095ff008, nsIAtom * aTag=0x012e38bc, int aNameSpaceID=6, nsStyleContext * aStyleContext=0x095ff0d0, nsFrameItems & aFrameItems={...}, int aXBLBaseTag=0)  Line 7424 + 0x53 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ConstructFrame(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x0997dd38, nsIFrame * aParentFrame=0x095ff008, nsFrameItems & aFrameItems={...})  Line 7379 + 0x35 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ProcessInlineChildren(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x09945ef0, nsIFrame * aFrame=0x095ff008, int aCanHaveGeneratedContent=1, nsFrameItems & aFrameItems={...}, int * aKidsAllInline=0x0012ede4)  Line 12618 + 0x3a bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ConstructInline(nsFrameConstructorState & aState={...}, const nsStyleDisplay * aDisplay=0x095fecf0, nsIContent * aContent=0x09945ef0, nsIFrame * aParentFrame=0x095fe714, nsStyleContext * aStyleContext=0x095fefb8, int aIsPositioned=0, nsIFrame * aNewFrame=0x095ff008)  Line 12390 + 0x21 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ConstructFrameByDisplayType(nsFrameConstructorState & aState={...}, const nsStyleDisplay * aDisplay=0x095fecf0, nsIContent * aContent=0x09945ef0, int aNameSpaceID=6, nsIAtom * aTag=0x012e38c8, nsIFrame * aParentFrame=0x095fe714, nsStyleContext * aStyleContext=0x095fefb8, nsFrameItems & aFrameItems={...}, int aHasPseudoParent=0)  Line 6591 + 0x22 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ConstructFrameInternal(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x09945ef0, nsIFrame * aParentFrame=0x095fe714, nsIAtom * aTag=0x012e38c8, int aNameSpaceID=6, nsStyleContext * aStyleContext=0x095fefb8, nsFrameItems & aFrameItems={...}, int aXBLBaseTag=0)  Line 7558 + 0x34 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ConstructFrame(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x09945ef0, nsIFrame * aParentFrame=0x095fe714, nsFrameItems & aFrameItems={...})  Line 7379 + 0x35 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ContentInserted(nsIContent * aContainer=0x05ce6050, nsIContent * aChild=0x09945ef0, int aIndexInContainer=0, nsILayoutHistoryState * aFrameState=0x05d58458)  Line 8966	C++
 	gklayout.dll!nsCSSFrameConstructor::RecreateFramesForContent(nsIContent * aContent=0x09945ef0)  Line 11087 + 0x25 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ProcessRestyledFrames(nsStyleChangeList & aChangeList={...})  Line 9825	C++
 	gklayout.dll!PresShell::RecreateFramesFor(nsIContent * aContent=0x09945ef0)  Line 3374 + 0x12 bytes	C++
 	gklayout.dll!nsXBLBindingRequest::DocumentLoaded(nsIDocument * aBindingDoc=0x09738080)  Line 263	C++
 	gklayout.dll!nsXBLStreamListener::Load(nsIDOMEvent * aEvent=0x0964b250)  Line 478	C++
 	gklayout.dll!DispatchToInterface(nsIDOMEvent * aEvent=0x0964b250, nsIDOMEventListener * aListener=0x097924d4, unsigned int (nsIDOMEvent *)* aMethod=0x045c3de0, const nsID & aIID={...})  Line 184 + 0xb bytes	C++
 	gklayout.dll!nsEventListenerManager::HandleEvent(nsPresContext * aPresContext=0x00000000, nsEvent * aEvent=0x0012f624, nsIDOMEvent * * aDOMEvent=0x0012f59c, nsISupports * aCurrentTarget=0x09738080, unsigned int aFlags=6, nsEventStatus * aEventStatus=0x0012f5a0)  Line 1182 + 0x22 bytes	C++
 	gklayout.dll!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6)  Line 212	C++
 	gklayout.dll!nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6, nsDispatchingCallback * aCallback=0x00000000)  Line 271	C++
 	gklayout.dll!nsEventDispatcher::Dispatch(nsISupports * aTarget=0x09738080, nsPresContext * aPresContext=0x00000000, nsEvent * aEvent=0x0012f624, nsIDOMEvent * aDOMEvent=0x00000000, nsEventStatus * aEventStatus=0x00000000, nsDispatchingCallback * aCallback=0x00000000)  Line 479 + 0x12 bytes	C++
 	gklayout.dll!nsXMLDocument::EndLoad()  Line 580 + 0x15 bytes	C++
 	gklayout.dll!nsXMLContentSink::DidBuildModel()  Line 381	C++
 	gkparser.dll!nsExpatDriver::DidBuildModel(unsigned int anErrorCode=0, int aNotifySink=1, nsIParser * aParser=0x05dc1758, nsIContentSink * aSink=0x05cc6520)  Line 1311 + 0xe bytes	C++
 	gkparser.dll!nsParser::DidBuildModel(unsigned int anErrorCode=0)  Line 1527 + 0x35 bytes	C++
 	gkparser.dll!nsParser::ResumeParse(int allowIteration=1, int aIsFinalChunk=1, int aCanInterrupt=1)  Line 2283	C++
 	gkparser.dll!nsParser::OnStopRequest(nsIRequest * request=0x097b3ca8, nsISupports * aContext=0x00000000, unsigned int status=0)  Line 2916 + 0x17 bytes	C++
 	gklayout.dll!nsXBLStreamListener::OnStopRequest(nsIRequest * request=0x097b3ca8, nsISupports * aCtxt=0x00000000, unsigned int aStatus=0)  Line 380 + 0x28 bytes	C++
 	necko.dll!nsBaseChannel::OnStopRequest(nsIRequest * request=0x097ddf80, nsISupports * ctxt=0x00000000, unsigned int status=0)  Line 623	C++
 	necko.dll!nsInputStreamPump::OnStateStop()  Line 577	C++
 	necko.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x0863dbc8)  Line 401 + 0xb bytes	C++
 	xpcom_core.dll!nsInputStreamReadyEvent::Run()  Line 112	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012f848)  Line 511	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x012d3fe8, int mayWait=1)  Line 227 + 0x16 bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 170 + 0xc bytes	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 182 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc=1, char * * argv=0x003fffe0, const nsXREAppData * aAppData=0x012d0880)  Line 3265 + 0x25 bytes	C++
 	firefox.exe!NS_internal_main(int argc=1, char * * argv=0x003fffe0)  Line 156 + 0x12 bytes	C++
 	firefox.exe!wmain(int argc=1, unsigned short * * argv=0x003fa2e8)  Line 87 + 0xd bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 583 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 403	C
 	kernel32.dll!_BaseProcessStart@4()  + 0x23 bytes
(Reporter)

Updated

10 years ago
Flags: blocking1.9.1?
Not blocking, but this is an exploitable crash. Marking security sensitive.
Group: core-security
Flags: blocking1.9.1? → blocking1.9.1-
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [sg:critical]
(Reporter)

Updated

10 years ago
Flags: wanted1.9.1?
Flags: blocking1.9.2?
(Assignee)

Updated

10 years ago
Assignee: nobody → Olli.Pettay
Component: DOM → XBL
QA Contact: general → xbl
(Assignee)

Comment 2

10 years ago
Created attachment 353458 [details] [diff] [review]
proposed patch
Attachment #353458 - Flags: superreview?(bzbarsky)
Attachment #353458 - Flags: review?(bzbarsky)
Comment on attachment 353458 [details] [diff] [review]
proposed patch

Looks ok, though it woud be even better to have a helper function that we call both here and in binding teardown (where we have to do the same thing, right?)
Attachment #353458 - Flags: superreview?(bzbarsky)
Attachment #353458 - Flags: superreview+
Attachment #353458 - Flags: review?(bzbarsky)
Attachment #353458 - Flags: review+
(Assignee)

Comment 4

10 years ago
I'll file a followup to add a helper and to make sure we call
undo install properly.
(Assignee)

Updated

10 years ago
Blocks: 470491
(Assignee)

Updated

10 years ago
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Reporter)

Comment 5

10 years ago
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081220 Minefield/3.2a1pre
Status: RESOLVED → VERIFIED
Do we need this fix in 3.0? bug 344258 was not landed on the 1.9.0 branch but the nsXBLBinding::GenerateAnonymousContent code this patch touches is the same.
Flags: wanted1.9.0.x?
Whiteboard: [sg:critical] → [sg:critical?]
I'll assume yes for now so we don't lose track.
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.6?
(Assignee)

Updated

10 years ago
Attachment #353458 - Flags: approval1.9.1?
Comment on attachment 353458 [details] [diff] [review]
proposed patch

a191=beltzner
Attachment #353458 - Flags: approval1.9.1? → approval1.9.1+
(Assignee)

Updated

10 years ago
Flags: wanted1.9.1?
Flags: blocking1.9.2?
Keywords: fixed1.9.1
(Assignee)

Updated

10 years ago
Attachment #353458 - Flags: approval1.9.0.6?
Flags: blocking1.9.0.6? → blocking1.9.0.6+
Comment on attachment 353458 [details] [diff] [review]
proposed patch

Approved for 1.9.0.6, a=dveditz for release-drivers.
Attachment #353458 - Flags: approval1.9.0.6? → approval1.9.0.6+
(Assignee)

Comment 10

10 years ago
The problem with this is to verify this on 1.9.0.x because the testcase is
for 1.9.1.
Olli: Any way to make a 1.9.0 testcase? Regardless, if this makes us safer, we should take it.
(Assignee)

Comment 12

10 years ago
Trying to create 1.9.0 testcase...

Martijn, perhaps you have some suggestions?
(Reporter)

Comment 13

10 years ago
Sorry, I don't know of a way to make it crash in 1.9.0, the original testcase doesn't crash in 1.9.0 either.
(Reporter)

Comment 14

10 years ago
However I did find a new case that has a similar stacktrace as this bug, I filed bug 472212 for it.
(Assignee)

Updated

10 years ago
Keywords: fixed1.9.0.6

Comment 15

10 years ago
1.9+ regression, not wanted on 1.8.
Flags: wanted1.8.1.x-
Flags: wanted1.8.0.x-
So there is no real way to verify this fix for 1.9.0.6? I double-checked and it doesn't crash in Firefox 3.0.5.
(Reporter)

Comment 17

10 years ago
Yes, afaik, there is no way to verify this fix for 1.9.0.6.
Group: core-security
Whiteboard: [sg:critical?] → [sg:critical?] post 1.8-branch
Verified fixed on the 1.9.1 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090225 Firefox and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090225 Shiretoko/3.1b3pre.
Keywords: fixed1.9.1 → verified1.9.1
Crash Signature: [@ nsContentUtils::ComparePosition]
You need to log in before you can comment on or make changes to this bug.