Closed
Bug 1282746
Opened 7 years ago
Closed 7 years ago
Assertion failure: comp == compartment || runtime()->isAtomsCompartment(comp) || (srcKind == JS::TraceKind::Object && InCrossCompartmentMap(static_cast<JSObject*>(src), thing)), at js/src/jsgc.cpp:3745
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: decoder, Assigned: efaust)
References
Details
(5 keywords, Whiteboard: [jsbugmon:][post-critsmash-triage][adv-main49+])
Attachments
(1 file)
842 bytes,
patch
|
Waldo
:
review+
gchang
:
approval-mozilla-aurora+
gchang
:
approval-mozilla-beta+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision d87b76177b2f (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe): function runTestCase(testcase) {} var lfGlobal = newGlobal(); for (lfLocal in this) { if (!(lfLocal in lfGlobal)) { lfGlobal[lfLocal] = this[lfLocal]; } } lfGlobal.offThreadCompileScript(` var p = new Proxy({}, {}); runTestCase.prototype.__proto__ = p; fullcompartmentchecks(true); `); lfGlobal.runOffThreadScript(); Backtrace: received signal SIGSEGV, Segmentation fault. 0x0000000000975cd0 in CompartmentCheckTracer::onChild (this=this@entry=0x7fffffffcfd0, thing=...) at js/src/jsgc.cpp:3743 #0 0x0000000000975cd0 in CompartmentCheckTracer::onChild (this=this@entry=0x7fffffffcfd0, thing=...) at js/src/jsgc.cpp:3743 #1 0x0000000000d57c5c in JS::CallbackTracer::onShapeEdge (shapep=0x7ffff7e74648, this=0x7fffffffcfd0) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/gcc/sanitizer/none/type/debug/dist/include/js/TracingAPI.h:131 #2 JS::CallbackTracer::dispatchToOnEdge (shapep=0x7ffff7e74648, this=0x7fffffffcfd0) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/gcc/sanitizer/none/type/debug/dist/include/js/TracingAPI.h:209 #3 DoCallback<js::Shape*> (trc=0x7fffffffcfd0, thingp=0x7ffff7e74648, name=<optimized out>) at js/src/gc/Tracer.cpp:51 #4 0x0000000000a09fb7 in js::ProxyObject::trace (trc=0x7fffffffcfd8, obj=0x7ffff7e74640) at js/src/proxy/Proxy.cpp:619 #5 0x000000000094546f in js::Class::doTrace (this=<optimized out>, obj=0x7ffff7e74640, trc=0x7fffffffcfd8) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/gcc/sanitizer/none/type/debug/dist/include/js/Class.h:815 #6 JSObject::traceChildren (this=0x7ffff7e74640, trc=0x7fffffffcfd8) at js/src/jsobj.cpp:3890 #7 0x0000000000d3b28a in js::TraceChildren (trc=trc@entry=0x7fffffffcfd8, thing=0x7ffff7e74640, kind=kind@entry=JS::TraceKind::Object) at js/src/gc/Tracer.cpp:126 #8 0x000000000093f0ad in js::gc::GCRuntime::checkForCompartmentMismatches (this=this@entry=0x7ffff6965608) at js/src/jsgc.cpp:3769 #9 0x0000000000976608 in js::gc::GCRuntime::checkForCompartmentMismatches (this=0x7ffff6965608) at js/src/jsgc.cpp:3811 #10 js::gc::GCRuntime::beginMarkPhase (this=this@entry=0x7ffff6965608, reason=reason@entry=JS::gcreason::DESTROY_RUNTIME, lock=...) at js/src/jsgc.cpp:3799 #11 0x000000000097b1e6 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0x7ffff6965608, budget=..., reason=reason@entry=JS::gcreason::DESTROY_RUNTIME, lock=...) at js/src/jsgc.cpp:5919 #12 0x000000000097c6ef in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff6965608, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:6204 #13 0x000000000097cdb6 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff6965608, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:6312 #14 0x000000000097d1ab in js::gc::GCRuntime::gc (this=this@entry=0x7ffff6965608, gckind=gckind@entry=GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:6379 #15 0x0000000000b289cf in JSRuntime::destroyRuntime (this=this@entry=0x7ffff69651c8) at js/src/vm/Runtime.cpp:430 #16 0x0000000000910569 in JSContext::~JSContext (this=0x7ffff6965000, __in_chrg=<optimized out>) at js/src/jscntxt.cpp:892 #17 0x00000000009108ab in js_delete_poison<JSContext> (p=0x7ffff6965000) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/gcc/sanitizer/none/type/debug/dist/include/js/Utility.h:392 #18 js::DestroyContext (cx=0x7ffff6965000) at js/src/jscntxt.cpp:133 #19 0x0000000000896bf7 in JS_DestroyRuntime (rt=rt@entry=0x7ffff69651c8) at js/src/jsapi.cpp:476 #20 0x0000000000438578 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:7452 rax 0x0 0 rbx 0x7fffffffcd30 140737488342320 rcx 0x7ffff6c28a2d 140737333332525 rdx 0x0 0 rsi 0x7ffff6ef7770 140737336276848 rdi 0x7ffff6ef6540 140737336272192 rbp 0x7fffffffcdd0 140737488342480 rsp 0x7fffffffcce0 140737488342240 r8 0x7ffff6ef7770 140737336276848 r9 0x7ffff7fdc740 140737353992000 r10 0x58 88 r11 0x7ffff6b9f750 140737332770640 r12 0x7ffff7ed1718 140737352898328 r13 0x7ffff7e74640 140737352517184 r14 0xfffa7fffffffffff -1548112371908609 r15 0xfffc000000000000 -1125899906842624 rip 0x975cd0 <CompartmentCheckTracer::onChild(JS::GCCellPtr const&)+864> => 0x975cd0 <CompartmentCheckTracer::onChild(JS::GCCellPtr const&)+864>: movl $0x0,0x0 0x975cdb <CompartmentCheckTracer::onChild(JS::GCCellPtr const&)+875>: ud2 This might be a dup to one of the previous GC bugs on file, but I'm not sure because the assertion here is new. Marking s-s because GC is involved.
Reporter | ||
Comment 1•7 years ago
|
||
Maybe bug 1275080 because it also involves Proxy?
Updated•7 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 2•7 years ago
|
||
JSBugMon: Bisection requested, result: === Treeherder Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20160427220852" and the hash "37c815005a7223bb81f947957bd80ae45c26376f". The "bad" changeset has the timestamp "20160427224854" and the hash "3c4b7e1de6290ef6e21f2f9e17f99ee5a04f47c6". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=37c815005a7223bb81f947957bd80ae45c26376f&tochange=3c4b7e1de6290ef6e21f2f9e17f99ee5a04f47c6
Comment 3•7 years ago
|
||
Sounds like a compartment mismatch, so I'm marking this sec-high. The regression window blames Waldo.
Flags: needinfo?(jwalden+bmo)
Keywords: sec-high
Assignee | ||
Comment 5•7 years ago
|
||
This test is pretty targeted. I suggest we hold off on landing it for a while.
Assignee: nobody → efaustbmo
Status: NEW → ASSIGNED
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(efaustbmo)
Attachment #8773051 -
Flags: review?(jwalden+bmo)
Comment 6•7 years ago
|
||
Comment on attachment 8773051 [details] [diff] [review] Fix Review of attachment 8773051 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this makes sense. Don't know why I wasn't seeing this immediately when looking at this in bug 1280246, and bug 1275080. :-( (I do think the proposed fix in bug 1275080, which would have addressed the problem as well, is also something we should do -- but it's not critical if we make this change, which routes around the problem in similar fashion.)
Attachment #8773051 -
Flags: review?(jwalden+bmo) → review+
Comment 9•7 years ago
|
||
There's memory-unsafety in them thar hills.
Keywords: sec-high → sec-critical
Comment 10•7 years ago
|
||
[Tracking Requested - why for this release]: Carrying over tracking flags from dupe Bug 1275080. There was an open question in the other bug as to whether this affects 48 and 45esr.
status-firefox49:
--- → affected
status-firefox-esr45:
--- → ?
tracking-firefox48:
--- → ?
tracking-firefox49:
--- → +
tracking-firefox50:
--- → +
Assignee | ||
Comment 11•7 years ago
|
||
Comment on attachment 8773051 [details] [diff] [review] Fix [Security approval request comment] How easily could an exploit be constructed based on the patch? It's not all that complicated to discover the problem, based on the patch. It involves a stray memory write and incorrect object flags, but the particular object being set is probably not actually exploitable? Still, those side effects are pretty scary. The test, which is basically the minimal operations set to hit the bug, will not land, immediately. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Nope. Which older supported branches are affected by this flaw? 47+ If not all supported branches, which bug introduced the flaw? Bug 888969. The bug was closed for milestone 49, but it was leave-open for a while. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? This patch should apply cleanly. How likely is this patch to cause regressions; how much testing does it need? Very unlikely. This just correctly handles what was an API violation to avoid a semi-random write.
Attachment #8773051 -
Flags: sec-approval?
Updated•7 years ago
|
Updated•7 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:]
Comment 12•7 years ago
|
||
JSBugMon: Cannot process bug: Error: Unsupported branch "47 Branch" required by bug
Comment 13•7 years ago
|
||
This is too late for the current release cycle so it will have to land two weeks into the next. I'm giving it sec-approval+ for checkin on August 16 (not before). We'll want patches for affected branches made and nominated at the same time as well.
tracking-firefox48:
? → ---
Whiteboard: [jsbugmon:] → [jsbugmon:][checkin on 8/16]
Updated•7 years ago
|
Attachment #8773051 -
Flags: sec-approval? → sec-approval+
Updated•7 years ago
|
status-firefox51:
--- → affected
tracking-firefox51:
--- → +
Reporter | ||
Comment 14•7 years ago
|
||
Eric, can we land this today? That would greatly help me triaging other bugs with similar signatures. Thanks!
Flags: needinfo?(efaustbmo)
Comment 15•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcfbc324f389
Flags: needinfo?(efaustbmo)
Whiteboard: [jsbugmon:][checkin on 8/16] → [jsbugmon:]
https://hg.mozilla.org/mozilla-central/rev/fcfbc324f389
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Updated•7 years ago
|
Group: javascript-core-security → core-security-release
Can you also request uplift as Al mentioned, for aurora and beta? We are heading into beta 8 next Monday. Thanks.
Flags: needinfo?(efaustbmo)
This can still make beta 9. Can you request uplift? It's sec-critical and we already landed the fix on m-c two weeks ago.
Flags: needinfo?(nihsanullah)
Flags: needinfo?(jwalden+bmo)
Assignee | ||
Comment 20•7 years ago
|
||
Comment on attachment 8773051 [details] [diff] [review] Fix Approval Request Comment [Feature/regressing bug #]: Bug 888969 [User impact if declined]: sec-crit random write. [Describe test coverage new/current, TreeHerder]: Landed on central 2 weeks ago. No negative reports. [Risks and why]: Small, no new behavior, just fixes an API violation. [String/UUID change made/needed]: None.
Flags: needinfo?(nihsanullah)
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(efaustbmo)
Attachment #8773051 -
Flags: approval-mozilla-beta?
Attachment #8773051 -
Flags: approval-mozilla-aurora?
Comment 21•7 years ago
|
||
Comment on attachment 8773051 [details] [diff] [review] Fix Review of attachment 8773051 [details] [diff] [review]: ----------------------------------------------------------------- This patch fixes a sec-critical issue. Take it in aurora and 49 beta.
Attachment #8773051 -
Flags: approval-mozilla-beta?
Attachment #8773051 -
Flags: approval-mozilla-beta+
Attachment #8773051 -
Flags: approval-mozilla-aurora?
Attachment #8773051 -
Flags: approval-mozilla-aurora+
Comment 22•7 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/20b2e73b19df
Flags: in-testsuite?
Updated•7 years ago
|
Whiteboard: [jsbugmon:] → [jsbugmon:][post-critsmash-triage]
Updated•7 years ago
|
Whiteboard: [jsbugmon:][post-critsmash-triage] → [jsbugmon:][post-critsmash-triage][adv-main49+]
You need to log in
before you can comment on or make changes to this bug.
Description
•