Closed
Bug 1084656
Opened 10 years ago
Closed 10 years ago
Assertion failure: !JS_IsExceptionPending(cx), at ./EventBinding.cpp:182
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: MatsPalmgren_bugz, Assigned: bholley)
Details
(Keywords: reproducible, sec-other, testcase, Whiteboard: [adv-main36-])
Attachments
(3 files)
I don't know if this is a security issue, but hiding it just in case... STR 0. unzip the attached file 1. load test/817219.html in a FF debug build 2. wait a few seconds 3. type F12 (open devtools) 4. wait a few seconds => fatal assertion
Reporter | ||
Comment 1•10 years ago
|
||
Comment 2•10 years ago
|
||
Note, without devtools there is 817219-iframe.html, line 26: too much recursion
Comment 3•10 years ago
|
||
Looks like we have the exception pending even before we call Event::GetType
Comment 4•10 years ago
|
||
js::Proxy::getPrototypeOf calls xpc::XrayWrapper<js::CrossCompartmentWrapper, xpc::DOMXrayTraits>::getPrototypeOf calls xpc::XrayTraits::getExpandoObject calls xpc::XrayTraits::getExpandoObjectInternal calls JS_WrapObject which throws in JSCompartment::wrap because of: JS_CHECK_CHROME_RECURSION(cx, return false); but then getExpandoObjectInternal drops the exception on the floor, and off we are, running JS on a cx with an exception pending.
Component: DOM → XPConnect
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8508179 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 6•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9b0b533bf17c
Comment 7•10 years ago
|
||
Comment on attachment 8508179 [details] [diff] [review] Properly propagate exceptions out of getExpandoObjectInternal. v1 > XrayTraits::attachExpandoObject(JSContext *cx, HandleObject target, Can you assert no pending exception on cx before the getExpandoObjectInternal call? >+ RootedObject expando(cx, nullptr); (several places, with different variable names). The default value of RootedObject is nullptr, no? r=me
Attachment #8508179 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #7) > Can you assert no pending exception on cx before the > getExpandoObjectInternal call? Sure. > (several places, with different variable names). The default value of > RootedObject is nullptr, no? At one point Waldo convinced me that EIBTI for this stuff, but I think the pattern is now common enough that it doesn't make sense anymore. https://hg.mozilla.org/integration/mozilla-inbound/rev/43c4600ab4ce
Comment 9•10 years ago
|
||
Bobby, what security rating should this get? I don't understand what the security implications are for this.
Flags: needinfo?(bobbyholley)
Assignee | ||
Comment 10•10 years ago
|
||
In general I doubt it's a security issue. Jorendorff, are you aware of any common security issues arising from failing to propagate exceptions like over-recursion?
Flags: needinfo?(bobbyholley) → needinfo?(jorendorff)
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/43c4600ab4ce
Assignee: nobody → bobbyholley
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox36:
--- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 12•10 years ago
|
||
This went in with no security rating. How far back did this issue go?
Updated•9 years ago
|
status-firefox35:
--- → wontfix
status-firefox-esr31:
--- → wontfix
Updated•9 years ago
|
Whiteboard: [adv-main36-]
Updated•9 years ago
|
Group: core-security → core-security-release
Comment 13•8 years ago
|
||
(not that this still matters, a year later) There are no known security issues involving this assertion, no. It's there to catch bugs.
Flags: needinfo?(jorendorff)
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•