Closed Bug 1084656 Opened 10 years ago Closed 10 years ago

Assertion failure: !JS_IsExceptionPending(cx), at ./EventBinding.cpp:182

Categories

(Core :: XPConnect, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox35 --- wontfix
firefox36 --- fixed
firefox-esr31 --- wontfix

People

(Reporter: MatsPalmgren_bugz, Assigned: bholley)

Details

(Keywords: reproducible, sec-other, testcase, Whiteboard: [adv-main36-])

Attachments

(3 files)

Attached file test.zip
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
Attached file stack
Note, without devtools there is 
817219-iframe.html, line 26: too much recursion
Looks like we have the exception pending even before we call Event::GetType
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
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+
(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
Bobby, what security rating should this get?  I don't understand what the security implications are for this.
Flags: needinfo?(bobbyholley)
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)
https://hg.mozilla.org/mozilla-central/rev/43c4600ab4ce
Assignee: nobody → bobbyholley
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
This went in with no security rating. How far back did this issue go?
Keywords: sec-other
Whiteboard: [adv-main36-]
Group: core-security → core-security-release
(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)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: