Closed
Bug 1313810
Opened 6 years ago
Closed 6 years ago
Compartment mismatch or Assertion failure: cx->isInsideCurrentCompartment(proto.toObject()), at js/src/vm/ObjectGroup.cpp:470
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: gkw, Assigned: till)
References
Details
(4 keywords, Whiteboard: [jsbugmon:update])
Attachments
(2 files)
35.23 KB,
text/plain
|
Details | |
4.41 KB,
patch
|
arai
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 1561c917ee27 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --ion-eager): // jsfunfuzz-generated toString = (function z() { return function() { Promise = evalcx('lazy').Promise; }; })(); [].join(this, toString); // Adapted from randomly chosen test: js/src/tests/ecma_6/Promise/promise-subclassing.js class SubPromise extends Promise {} SubPromise.all([new Promise(function(){})]); Backtrace: 0 js-dbg-64-dm-clang-darwin-1561c917ee27 0x000000010254cf0a js::ObjectGroup::defaultNewGroup(js::ExclusiveContext*, js::Class const*, js::TaggedProto, JSObject*) + 3226 (ObjectGroup.cpp:470) 1 js-dbg-64-dm-clang-darwin-1561c917ee27 0x0000000102341533 js::NewObjectWithGivenTaggedProto(js::ExclusiveContext*, js::Class const*, JS::Handle<js::TaggedProto>, js::gc::AllocKind, js::NewObjectKind, unsigned int) + 243 (RootingAPI.h:802) 2 js-dbg-64-dm-clang-darwin-1561c917ee27 0x00000001026d1b49 CreatePromiseObjectInternal(JSContext*, JS::Handle<JSObject*>, bool) + 217 (jsobjinlines.h:744) 3 js-dbg-64-dm-clang-darwin-1561c917ee27 0x00000001026d3222 js::PromiseObject::create(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>) + 210 (RootingAPI.h:802) 4 js-dbg-64-dm-clang-darwin-1561c917ee27 0x00000001026d4a91 js::PromiseConstructor(JSContext*, unsigned int, JS::Value*) + 817 (RootingAPI.h:802) 5 js-dbg-64-dm-clang-darwin-1561c917ee27 0x000000010251236d js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 93 (jscntxtinlines.h:240) /snip For detailed crash information, see attachment. Setting s-s because on opt builds, the following message shows: *** Compartment mismatch 0x7fa0f1d48ac0 vs. 0x7fa0f1d3bce0 Segmentation fault: 11
![]() |
Reporter | |
Comment 1•6 years ago
|
||
![]() |
Reporter | |
Comment 2•6 years ago
|
||
=== Treeherder Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20160716060553" and the hash "a80fdfc128b0c29dc34dd3fc28868252111a5d52". The "bad" changeset has the timestamp "20160716064053" and the hash "a97ebab940c6ba0f19348c38d17a2193ee75c9ab". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a80fdfc128b0c29dc34dd3fc28868252111a5d52&tochange=a97ebab940c6ba0f19348c38d17a2193ee75c9ab Till, is bug 1242662 a likely regressor?
Blocks: 1242662
Flags: needinfo?(till)
Assignee | ||
Comment 3•6 years ago
|
||
> Till, is bug 1242662 a likely regressor? No, this is caused by bug 911216. I'm not sure why the regression range is off. Here's a simpler test case: class SubPromise extends newGlobal().Promise {} new SubPromise(()=>{}); Patch coming up.
Flags: needinfo?(till)
Assignee | ||
Comment 4•6 years ago
|
||
Turns out the way xray wrapper handling was set up for Promises didn't work for normal CCWs. I don't think this is s-s: it crashes release builds in a predictable, if unfortunate, way, but shouldn't be exploitable.
Comment 5•6 years ago
|
||
Comment on attachment 8805768 [details] [diff] [review] Properly handle Promise-subclassing with a cross-compartment Promise superclass Review of attachment 8805768 [details] [diff] [review]: ----------------------------------------------------------------- looks good. as talked in IRC, we could remove wrap and unwrap for needsWrapping==true case, as it's used only in that case, and wrapped value isn't used. but it's about API design (what should be wrapped and not), we could do in followup bug if necessary. ::: js/src/jit-test/tests/promise/promise-cross-compartment-subclassing.js @@ +1,2 @@ > +global = newGlobal(); > +OtherPromise = global.Promise; I'd prefer having var or let there.
Attachment #8805768 -
Flags: review?(arai.unmht) → review+
![]() |
Reporter | |
Comment 6•6 years ago
|
||
(In reply to Till Schneidereit [:till] from comment #4) > I don't think this is s-s: it crashes release builds in a > predictable, if unfortunate, way, but shouldn't be exploitable. Opening up.
Group: javascript-core-security
Pushed by tschneidereit@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0cc058bf7a5b Properly handle Promise-subclassing with a cross-compartment Promise superclass. r=arai
Comment 8•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0cc058bf7a5b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•