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

VERIFIED FIXED in Firefox 22

Status

()

--
critical
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: jruderman, Assigned: bzbarsky)

Tracking

(Blocks: 1 bug, {crash, regression, testcase})

22 Branch
mozilla23
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox21 unaffected, firefox22+ fixed, firefox23+ verified)

Details

(crash signature)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 738269 [details]
testcase (crashes Firefox when loaded)
(Reporter)

Comment 1

6 years ago
Created attachment 738271 [details]
stack (gdb)
(Assignee)

Comment 2

6 years ago
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.
Blocks: 825628
Keywords: regression
(Assignee)

Updated

6 years ago
tracking-firefox22: --- → ?
tracking-firefox23: --- → ?
(Assignee)

Comment 3

6 years ago
Created attachment 738287 [details] [diff] [review]
When we have named constructors, make sure we managed to set up an interface object before looking for them.
Attachment #738287 - Flags: review?(peterv)
(Assignee)

Updated

6 years ago
Assignee: nobody → bzbarsky
(Assignee)

Comment 4

6 years ago
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.
Attachment #738287 - Flags: approval-mozilla-aurora?

Comment 5

6 years ago
On Windows: bp-8130bcaa-3d36-48b8-bbf2-e3cd42130417
Crash Signature: [@ mozilla::dom::HTMLImageElementBinding::DefineDOMInterface] → [@ mozilla::dom::HTMLImageElementBinding::DefineDOMInterface] [@ js::shadow::Object::slotRef(unsigned int)] [@ js::GetObjectClass(JSObject*)]
status-firefox21: --- → unaffected
status-firefox22: --- → affected
status-firefox23: --- → affected
OS: Mac OS X → All
Hardware: x86_64 → All
Version: Trunk → 22 Branch
Attachment #738287 - Flags: review?(peterv) → review+
(Assignee)

Comment 6

6 years ago
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.
(Assignee)

Comment 7

6 years ago
Created attachment 738600 [details] [diff] [review]
With test that passes in our harnesses
(Assignee)

Updated

6 years ago
Attachment #738287 - Attachment is obsolete: true
Attachment #738287 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

6 years ago
Attachment #738600 - Flags: approval-mozilla-aurora?
Attachment #738600 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
tracking-firefox22: ? → +
tracking-firefox23: ? → +
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.
Attachment #738600 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora?
(Assignee)

Comment 9

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b83cfe95da3
Flags: in-testsuite+
Target Milestone: --- → mozilla23
https://hg.mozilla.org/mozilla-central/rev/4b83cfe95da3
Status: NEW → RESOLVED
Last Resolved: 6 years ago
status-firefox23: affected → fixed
Resolution: --- → FIXED

Updated

6 years ago
Attachment #738600 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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).
Status: RESOLVED → VERIFIED
status-firefox23: fixed → verified
You need to log in before you can comment on or make changes to this bug.