389694 - Crash [@ nsImageDocument::RestoreImage()] with DOMAttrModified on image document, clicking and removing window

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, when clicking on the black image in the iframe, Mozilla crashes.
Also happens on branch, so marking security sensitive for now.

http://crash-stats.mozilla.com/report/index/d5efc47d-3b9c-11dc-b20b-001a4bd43e5c
0  	nsImageDocument::RestoreImage()
1 	nsImageDocument::RestoreImageTo(int,int)
2 	nsImageDocument::HandleEvent(nsIDOMEvent *)
3 	nsEventListenerManager::HandleEventSubType(nsListenerStruct *,nsIDOMEventListener *,nsIDOMEvent *,nsISupports *,unsigned int)
4 	nsEventListenerManager::HandleEvent(nsPresContext *,nsEvent *,nsIDOMEvent * *,nsISupports *,unsigned int,nsEventStatus *)
5 	nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor &,unsigned int)
6 	nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor &,unsigned int,nsDispatchingCallback *)
7 	nsEventDispatcher::Dispatch(nsISupports *,nsPresContext *,nsEvent *,nsIDOMEvent *,nsEventStatus *,nsDispatchingCallback *)
8 	PresShell::HandleEventInternal(nsEvent *,nsIView *,nsEventStatus *)
9 	PresShell::HandleEventWithTarget(nsEvent *,nsIFrame *,nsIContent *,nsEventStatus *)

Comment 1

[Martijn Wargers (dead)]

Oops! The testcase is crashing directly now.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

It is a null pointer crash what I get.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Er, no, I got the null pointer crash when I tested the testcase you sent to me.
The testcase here does cause a SSensitive crash.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: proposed patch, fixes both crashes

Comment 5

[Martijn Wargers (dead)]

Hmm, bug 389663 seems to be the same, in some part?

Comment 6

[Eli Friedman]

In part, except my patch caused the crash to trigger in normal situations.

Comment 7

[Jeff Walden [:Waldo]]

Add a test at least for the null-deref, please, if it's not obvious how to get from that to the security-sensitive crash?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Because of bug 389663, I'll check in only the null-check fixes part of
the patch.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: only null-deref fixes

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 274977 [details] [diff] [review]
only null-deref fixes

a1.9=dbaron

Comment 11

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007080805 Minefield/3.0a8pre

Comment 12

[Daniel Veditz [:dveditz]]

The 1.8 branch seems to be only the null deref crash, and bug 389663 seems to be post 1.8. Is there a more dangerous crash on the 1.8 branch that I'm not seeing?