Closed Bug 386914 Opened 17 years ago Closed 17 years ago

Crash [@ nsXULDocument::ExecuteOnBroadcastHandlerFor] with DOMAttrModified event handler and observes


(Core :: XUL, defect)

Windows XP
Not set





(Reporter: martijn.martijn, Assigned: smaug)



(4 keywords, Whiteboard: [sg:critical?])

Crash Data


(3 files)

Attached file testcase
See testcase, which crashes Mozilla within 100ms.
It also crashes branch, so marking security sensitive for now.
0  	@0x2436041
1 	nsCOMPtr<nsIContent>::nsCOMPtr<nsIContent>(nsQueryInterface)
2 	nsXULDocument::ExecuteOnBroadcastHandlerFor(nsIContent *,nsIDOMElement *,nsIAtom *)
3 	nsXULDocument::AttributeChanged(nsIDocument *,nsIContent *,int,nsIAtom *,int)
4 	nsNodeUtils::AttributeChanged(nsIContent *,int,nsIAtom *,int)
5 	nsGenericElement::SetAttrAndNotify(int,nsIAtom *,nsIAtom *,nsAString_internal const &,nsAttrValue &,int,int,int)
6 	nsGenericElement::SetAttr(int,nsIAtom *,nsIAtom *,nsAString_internal const &,int)
7 	nsGenericElement::SetAttribute(nsAString_internal const &,nsAString_internal const &)
8 	NS_InvokeByIndex_P
9 	AutoJSSuspendRequest::SuspendRequest()

Branch talkback ID: TB33739202G

I think this might be related to bug 351347.
Though, in this case the crash is not because of that, but because 
BroadcastListener gets deleted, I think.
Patch coming.
Assignee: jag → Olli.Pettay
Patch is not coming yet. I need to think broadcaster a bit more.
There are other possible crashers there, I think.
Attached patch proposed patchSplinter Review
For this type of crash keeping broadcast listeners alive is needed.
Attachment #271019 - Flags: superreview?(jst)
Attachment #271019 - Flags: review?(jst)
This is basically the same as bug 351347, right?  Down to the stack, and the analysis, at least... ;)
Note, in particular, the comments in that bug explaining why calling SetAttr here is bad to start with.
Yes, looks like the same bug. The suggestion in comment 6 in that bug
sounds pretty similar to what I did.
Attachment #271019 - Flags: superreview?(jst)
Attachment #271019 - Flags: superreview+
Attachment #271019 - Flags: review?(jst)
Attachment #271019 - Flags: review+
Closed: 17 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Attached patch for 1.8Splinter Review
Attachment #272465 - Flags: approval1.8.1.6?
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre
deleted aListener passed to nsXULDocument::ExecuteOnBroadcastHandlerFor(), crashes and
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: [sg:critical?]
Flags: blocking1.8.1.7+
Comment on attachment 272465 [details] [diff] [review]
for 1.8

approved for, a=dveditz for release-drivers
Attachment #272465 - Flags: approval1.8.1.7? → approval1.8.1.7+
Keywords: fixed1.8.1.7
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2007083003 BonEcho/ and the testcase in this bug.
No crash on testcase - adding verified keyword.
Group: security
Comment on attachment 272465 [details] [diff] [review]
for 1.8

a=asac for

(same patch shipped by distros for some time)
Attachment #272465 - Flags: approval1.8.0.15+
fix committed to 1.8.0 branch
Keywords: fixed1.8.0.15
crash test landed
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsXULDocument::ExecuteOnBroadcastHandlerFor]
You need to log in before you can comment on or make changes to this bug.