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)

x86_64
Linux
defect
Not set
critical

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)

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
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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)
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
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)
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)
Okay, I can reproduce this.
Assignee: jcoppeard → jimb
Oh, sorry, misread. Thought jonco was asking me to fix it...
Assignee: jimb → jcoppeard
Attachment #8744918 - Flags: review?(jimb) → review+
Blocks: 1258436
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?
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?
Attachment #8744918 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/mozilla-central/rev/9892ac28ce0d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Group: javascript-core-security → core-security-release
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 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+
JSBugMon: This bug has been automatically verified fixed on Fx48
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.