862610 - Crash [@ mozilla::dom::HTMLImageElementBinding::DefineDOMInterface] with proto manipulation

Status: VERIFIED FIXED

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

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

Comment 1

[Jesse Ruderman]

Attachment: stack (gdb)

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is fun.  When we go to set up HTMLImageElement.prototype, the call to JS_DefineProperties ends up doing a LookupProperty (in js::DefineNativeProperty) which ends up doing ScriptedIndirectProxyHandler::getPropertyDescriptor on HTMLElement.prototype.__proto__ (which the testcase redefined to some random proxy with no handler), which of course throws.

So JS_DefineProperties throws, we unwind to mozilla::dom::HTMLImageElementBinding::DefineDOMInterface, get a null interfaceObject, but never null-check it before starting to look for named constructor stuff.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: When we have named constructors, make sure we managed to set up an interface object before looking for them.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 738287 [details] [diff] [review]
When we have named constructors, make sure we managed to set up an interface object before looking for them.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 825628
User impact if declined: Possible to trigger near-null-dereference crashes
Testing completed (on m-c, etc.): Passes attached test
Risk to taking this patch (and alternatives if risky): Very low risk: simple
   null-check. 
String or IDL/UUID changes made by this patch:  Nope.

Comment 5

[Scoobidiver (away)]

On Windows: bp-8130bcaa-3d36-48b8-bbf2-e3cd42130417

Comment 6

[Boris Zbarsky [:bzbarsky]]

This crashtest fails crashtest-ipc due to bug 862825.  I'll modify it to address that.

I thought about this a bit more, and the js engine behavior here is just wrong.  Filed bug 862848.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: With test that passes in our harnesses

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 738600 [details] [diff] [review]
With test that passes in our harnesses

too quick on the approval, will wait for m-c landing.

Comment 9

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b83cfe95da3

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4b83cfe95da3

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a0b77ef3b4c8

Comment 12

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0

Verified as fixed on Firefox 23 Beta 1 (buildID: 20130625125232) latest Aurora (buildID: 20130624004020) and latest Nightly (buildID: 20130625031238).