Closed
Bug 1266434
Opened 8 years ago
Closed 8 years ago
AddressSanitizer: heap-use-after-free [@ js::detail::HashTableEntry<js::ReadBarriered<js::GlobalObject*> const>::isLive] with READ of size 4 with Debugger
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox47 | --- | unaffected |
firefox48 | + | verified |
firefox49 | + | verified |
firefox-esr45 | --- | unaffected |
People
(Reporter: decoder, Assigned: jonco)
References
(Blocks 1 open bug)
Details
(5 keywords, Whiteboard: [jsbugmon:update])
Attachments
(1 file)
3.14 KB,
patch
|
jimb
:
review+
lizzard
:
approval-mozilla-aurora+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision ae7413abfa4d (build with --enable-gczeal --enable-optimize="-O2 -g" --enable-address-sanitizer --enable-posix-nspr-emulation --disable-jemalloc --disable-tests --disable-debug, run with --fuzzing-safe --thread-count=2 --baseline-eager): try { var g = newGlobal(); var dbg = new Debugger(g); function test(type, provocation) { dbg.onEnterFrame = function handleFirstFrame(f) { dbg.onEnterFrame = function handleSecondFrame(f) { dbg.onEnterFrame = function handleThirdFrame(f) { }; }; }; } test('global', 'evaluate(\'debugger; throw \\\'mud\\\';\');'); } catch(exc2) {} try { dbg.addDebuggee(g); g.eval("" + function f() { function g() { } }); var g = newGlobal(); g.evaluate("function f(x) { return x + 1; }"); schedulegc(10); var gw = dbg.addDebuggee(g); var s = dbg.findScripts(); } catch(exc217) {} Backtrace: ==3515==ERROR: AddressSanitizer: heap-use-after-free on address 0x615000030150 at pc 0x1387c85 bp 0x7fffd97fb670 sp 0x7fffd97fb668 READ of size 4 at 0x615000030150 thread T0 #0 0x1387c84 in js::detail::HashTableEntry<js::ReadBarriered<js::GlobalObject*> const>::isLive() const js/src/opt64asan/dist/include/js/HashTable.h:789 #1 0x1387c84 in js::HashSet<js::ReadBarriered<js::GlobalObject*>, js::MovableCellHasher<js::ReadBarriered<js::GlobalObject*> >, js::RuntimeAllocPolicy>::all() const js/src/opt64asan/dist/include/js/HashTable.h:977 #2 0x1387c84 in js::Debugger::ScriptQuery::matchAllDebuggeeGlobals() js/src/vm/Debugger.cpp:4129 #3 0x12f36e3 in js::Debugger::ScriptQuery::omittedQuery() js/src/vm/Debugger.cpp:3968 #4 0x12f36e3 in js::Debugger::findScripts(JSContext*, unsigned int, JS::Value*) js/src/vm/Debugger.cpp:4282 #5 0x14cd4d9 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:235 #6 0x14cd4d9 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:480 #7 0x84cdef in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) js/src/jit/BaselineIC.cpp:6116 0x615000030150 is located 336 bytes inside of 512-byte region [0x615000030000,0x615000030200) freed by thread T0 here: #0 0x4b438f in __interceptor_free /srv/repos/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64 #1 0x1370db7 in js_free(void*) js/src/opt64asan/dist/include/js/Utility.h:265 #2 0x1370db7 in js::RuntimeAllocPolicy::free_(void*) js/src/vm/Runtime.h:1933 #3 0x1370db7 in js::detail::HashTable<js::ReadBarriered<js::GlobalObject*> const, js::HashSet<js::ReadBarriered<js::GlobalObject*>, js::MovableCellHasher<js::ReadBarriered<js::GlobalObject*> >, js::RuntimeAllocPolicy>::SetOps, js::RuntimeAllocPolicy>::changeTableSize(int, js::detail::HashTable<js::ReadBarriered<js::GlobalObject*> const, js::HashSet<js::ReadBarriered<js::GlobalObject*>, js::MovableCellHasher<js::ReadBarriered<js::GlobalObject*> >, js::RuntimeAllocPolicy>::SetOps, js::RuntimeAllocPolicy>::FailureBehavior) js/src/opt64asan/dist/include/js/HashTable.h:1445 #4 0x12e42cc in js::Debugger::removeDebuggeeGlobal(js::FreeOp*, js::GlobalObject*, js::detail::HashTable<js::ReadBarriered<js::GlobalObject*> const, js::HashSet<js::ReadBarriered<js::GlobalObject*>, js::MovableCellHasher<js::ReadBarriered<js::GlobalObject*> >, js::RuntimeAllocPolicy>::SetOps, js::RuntimeAllocPolicy>::Enum*) js/src/vm/Debugger.cpp:3774 #5 0x12e4cd0 in js::Debugger::detachAllDebuggersFromGlobal(js::FreeOp*, js::GlobalObject*) js/src/vm/Debugger.cpp:2874 #6 0x1045a2e in JSCompartment::sweepGlobalObject(js::FreeOp*) js/src/jscompartment.cpp:686 #7 0x10e4364 in js::gc::GCRuntime::beginSweepingZoneGroup() js/src/jsgc.cpp:5245 #8 0x10e75c3 in js::gc::GCRuntime::beginSweepPhase(bool) js/src/jsgc.cpp:5428 #9 0x10ee836 in js::gc::GCRuntime::incrementalCollectSlice(js::SliceBudget&, JS::gcreason::Reason) js/src/jsgc.cpp:6151 #10 0x10f0522 in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget&, JS::gcreason::Reason) js/src/jsgc.cpp:6388 #11 0x10f196b in js::gc::GCRuntime::collect(bool, js::SliceBudget, JS::gcreason::Reason) js/src/jsgc.cpp:6496 #12 0x10f8c54 in js::gc::GCRuntime::gc(JSGCInvocationKind, JS::gcreason::Reason) js/src/jsgc.cpp:6556 #13 0x10f8c54 in js::gc::GCRuntime::runDebugGC() js/src/jsgc.cpp:7039 #14 0x18a4508 in js::gc::GCRuntime::gcIfNeededPerAllocation(JSContext*) js/src/gc/Allocator.cpp:28 #15 0x18b7e46 in JSContext::runtime() const js/src/gc/Allocator.cpp:55 #16 0x18b7e46 in JSScript* js::Allocate<JSScript, (js::AllowGC)1>(js::ExclusiveContext*) js/src/gc/Allocator.cpp:213 #17 0x11c80d3 in JSScript::Create(js::ExclusiveContext*, JS::Handle<JSObject*>, bool, JS::ReadOnlyCompileOptions const&, JS::Handle<JSObject*>, unsigned int, unsigned int) js/src/jsscript.cpp:2804 #18 0x180d49f in js::frontend::CompileLazyFunction(JSContext*, JS::Handle<js::LazyScript*>, char16_t const*, unsigned long) js/src/frontend/BytecodeCompiler.cpp:832 #19 0x10ab934 in JSFunction::createScriptForLazilyInterpretedFunction(JSContext*, JS::Handle<JSFunction*>) js/src/jsfun.cpp:1435 #20 0x1048b20 in JSFunction::getOrCreateScript(JSContext*) js/src/jsfun.h:413 #21 0x1048b20 in JSCompartment::needsDelazificationForDebugger() const js/src/jscompartment.cpp:1033 #22 0x1048b20 in JSCompartment::ensureDelazifyScriptsForDebugger(JSContext*) js/src/jscompartment.cpp:1047 #23 0x1387ea9 in js::Debugger::ScriptQuery::addCompartment(JSCompartment*) js/src/vm/Debugger.cpp:4106 #24 0x1387bfe in js::Debugger::ScriptQuery::matchAllDebuggeeGlobals() js/src/vm/Debugger.cpp:4130 #25 0x12f36e3 in js::Debugger::ScriptQuery::omittedQuery() js/src/vm/Debugger.cpp:3968 #26 0x12f36e3 in js::Debugger::findScripts(JSContext*, unsigned int, JS::Value*) js/src/vm/Debugger.cpp:4282 #27 0x14cd4d9 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:235 [...] #34 0x14d0202 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/vm/Interpreter.cpp:736 SUMMARY: AddressSanitizer: heap-use-after-free js/src/opt64asan/dist/include/js/HashTable.h:789 js::detail::HashTableEntry<js::ReadBarriered<js::GlobalObject*> const>::isLive() const Shadow bytes around the buggy address: 0x0c2a7fffe010: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x0c2a7fffe020: fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd 0x0c2a7fffe030: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd Shadow byte legend (one shadow byte represents 8 application bytes): Freed heap region: fd ==3515==ABORTING
Updated•8 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 1•8 years ago
|
||
JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/f66ab359f3ef user: Jon Coppeard date: Wed Mar 23 02:30:00 2016 -0400 summary: Bug 1258436 - Remove GC suppression in JSFunction::createScriptForLazilyInterpretedFunction. r=sfink This iteration took 263.751 seconds to run.
Jon, is bug 1258436 a likely regressor?
Flags: needinfo?(jcoppeard)
Comment 3•8 years ago
|
||
UAF is security-sensitive by default. The need for Debugger mitigates, but we don't know that all Debugger users won't tickle in just the right way, or web-content-provokably, so it's still a bad thing.
Group: javascript-core-security
Assignee | ||
Comment 4•8 years ago
|
||
ScriptQuery::addCompartment calls JSCompartment::ensureDelazifyScriptsForDebugger and now that JSFunction::createScriptForLazilyInterpretedFunction can GC, we're hitting a GC and modifiying a debugger's debuggees set while it's being traversed by ScriptQuery::matchAllDebuggeeGlobals.
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 5•8 years ago
|
||
The simple thing to do is to do the delazification in a separate phase after finding all the compartments we are interested in.
Assignee: nobody → jcoppeard
Attachment #8744918 -
Flags: review?(jimb)
Comment 7•8 years ago
|
||
Oh, sorry, misread. Thought jonco was asking me to fix it...
Assignee: jimb → jcoppeard
Updated•8 years ago
|
Attachment #8744918 -
Flags: review?(jimb) → review+
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8744918 [details] [diff] [review] bug1266434-debugger-delazification [Security approval request comment] How easily could an exploit be constructed based on the patch? Hard, depends on GC timing and use of debugger. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? 48 is affected. If not all supported branches, which bug introduced the flaw? Bug 1258436. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Same patch should apply. How likely is this patch to cause regressions; how much testing does it need? Unlikely to cause regressions.
Attachment #8744918 -
Flags: sec-approval?
Updated•8 years ago
|
Keywords: csectype-uaf,
sec-high
Comment 9•8 years ago
|
||
Giving sec-approval for Trunk (49). We'll want an Aurora (48) patch as well. Can you please make one or nominate the existing if it applies cleanly?
status-firefox47:
--- → unaffected
status-firefox49:
--- → affected
status-firefox-esr45:
--- → unaffected
tracking-firefox48:
--- → +
tracking-firefox49:
--- → +
Updated•8 years ago
|
Attachment #8744918 -
Flags: sec-approval? → sec-approval+
Comment 10•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9892ac28ce0d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Updated•8 years ago
|
Status: RESOLVED → VERIFIED
Comment 11•8 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Updated•8 years ago
|
Group: javascript-core-security → core-security-release
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8744918 [details] [diff] [review] bug1266434-debugger-delazification Approval Request Comment [Feature/regressing bug #]: Bug 1258436. [User impact if declined]: Possible security vulnerability. [Describe test coverage new/current, TreeHerder]: On m-c since 28th May. [Risks and why]: Low. [String/UUID change made/needed]: None.
Attachment #8744918 -
Flags: approval-mozilla-aurora?
Comment 13•8 years ago
|
||
Comment on attachment 8744918 [details] [diff] [review] bug1266434-debugger-delazification Fix for sec-high vulnerability, please uplift to aurora
Attachment #8744918 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•8 years ago
|
Comment 15•8 years ago
|
||
JSBugMon: This bug has been automatically verified fixed on Fx48
Updated•8 years ago
|
Group: core-security-release
Updated•4 years ago
|
Blocks: asan-maintenance
You need to log in
before you can comment on or make changes to this bug.
Description
•