Closed Bug 420429 Opened 17 years ago Closed 17 years ago

Crash [@ nsContentList::PopulateWith] with mutation event listener on window, cloned XML PI

Categories

(Core :: DOM: Core & HTML, defect, P2)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: crash, testcase, Whiteboard: [sg:critical] post 1.8-branch)

Crash Data

Attachments

(4 files, 2 obsolete files)

Attached file testcase (obsolete) (deleted) —
Loading the testcase triggers a crash. It appears exploitable. The crash only happens if the mutation event listener is on |window|. If it's on |document|, there is no crash. The mutation event listener does not appear to be called, but it has to be installed in order to trigger the bug. Sometimes, this assertion shows up before the crash: ###!!! ASSERTION: tearoff not empty in dtor: '!(GetInterface()||GetNative()||GetJSObject())', file /Users/jruderman/trunk/mozilla/js/src/xpconnect/src/xpcinlines.h, line 662
Attached file testcase (deleted) —
Oops, attached the wrong thing in comment 0.
Attachment #306679 - Attachment is obsolete: true
Attached file stack trace (deleted) —
Whiteboard: [sg:critical]
Severity: normal → critical
Flags: blocking1.9?
Attached file More stacks (deleted) —
The nsXMLProcessingInstruction constructor calls SetData(): http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/xml/content/src/nsXMLProcessingInstruction.cpp&rev=1.77&root=/cvsroot&mark=84#78 which calls SetTextInternal() with aNotify=PR_TRUE but since we do this from the constructor there is no strong ref yet so when the nsMutationEvent object is destroyed when we leave the scope here http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/base/src/nsGenericDOMDataNode.cpp&rev=3.250&root=/cvsroot&mark=486#440 'this' is deleted. The first stack shows the call to free(), the second the crash stack.
Assignee: nobody → mats.palmgren
Status: NEW → ASSIGNED
Attached patch Patch rev. 1 (deleted) — Splinter Review
Don't notify when setting the value from within the ctor. (and mute a warning)
Attachment #306979 - Flags: superreview?(jst)
Attachment #306979 - Flags: review?(jst)
Attached patch Patch rev. 2 (obsolete) (deleted) — Splinter Review
Alternative patch that still uses SetData() that notifies, but doing it in a separate step after we have NS_ADDREF'ed the object.
OS: Mac OS X → All
Hardware: PC → All
On the 1.8 branch I don't crash, I just get WARNING: waaah!, file c:/dev/ff2/mozilla/content/xul/document/src/nsXULPrototypeDocument.cpp, line 862
Flags: wanted1.8.1.x-
Whiteboard: [sg:critical] → [sg:critical] post 1.8-branch
I sort of want to do both patch versions. I see no reason to notify as there can't be anything yet that listens to the notification. But it's in general a nice code cleanup to not have the "clonetext" argument. Though now isn't really the time for cleanups, so maybe we should just take the rev 1 patch for now and land the other once we've branched for FF3.
Attachment #306979 - Flags: superreview?(jst)
Attachment #306979 - Flags: superreview+
Attachment #306979 - Flags: review?(jst)
Attachment #306979 - Flags: review+
This patch is ready to be landed, right?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Summary: Crash [@ nsContentList::PopulateWith] with mutation event listener on window → Crash [@ nsContentList::PopulateWith] with mutation event listener on window, cloned XML PI
Patch rev.1 + crashtest landed: mozilla/content/base/src/nsCommentNode.cpp 3.89 mozilla/content/xml/content/src/nsXMLProcessingInstruction.cpp 1.78 mozilla/content/xml/content/crashtest/420429.xul 1.1 mozilla/content/xml/content/crashtest/crashtests.list 1.1 mozilla/testing/crashtest/crashtests.list 1.32 Filed bug 421034 on the parameter cleanup. -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Attachment #306980 - Attachment is obsolete: true
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008031704 Minefield/3.0b5pre as well as the Win XP nighty. No crash using the testcase.
Status: RESOLVED → VERIFIED
Group: core-security
Crash Signature: [@ nsContentList::PopulateWith]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: