1315943 - AddressSanitizer: heap-buffer-overflow [@ js::frontend::FullParseHandler::nextLazyClosedOverBinding] with READ of size 8 with Debugger

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 908557c762f7 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-debug --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2, run with --fuzzing-safe):

var s = "{}";
for (var i = 0; i < 21; i++) s += s;
var g = newGlobal();
var dbg = Debugger(g);
dbg.onDebuggerStatement = function(frame) {
    var s = frame.eval("f").return.script;
};
g.eval("line0 = Error().lineNumber;\n" + "debugger;\n" + // line0 + 1
    "function f(i) {\n" + // line0 + 2
    s + // line0 + 3 ... line0 + where - 2
    "}\n");


Backtrace:

==32035==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200001caf8 at pc 0x00000068cbef bp 0x7fffe2f205b0 sp 0x7fffe2f205a8
READ of size 8 at 0x60200001caf8 thread T0
    #0 0x68cbee in js::frontend::FullParseHandler::nextLazyClosedOverBinding() js/src/frontend/FullParseHandler.h:921:16
    #1 0x68cbee in js::frontend::Parser<js::frontend::FullParseHandler>::propagateFreeNamesAndMarkClosedOverBindings(js::frontend::ParseContext::Scope&) js/src/frontend/Parser.cpp:1396
    #2 0x63b023 in js::frontend::Parser<js::frontend::FullParseHandler>::finishLexicalScope(js::frontend::ParseContext::Scope&, js::frontend::ParseNode*) js/src/frontend/Parser.cpp:1821:10
    #3 0x63b023 in js::frontend::Parser<js::frontend::FullParseHandler>::blockStatement(js::frontend::YieldHandling, unsigned int) js/src/frontend/Parser.cpp:4268
    #4 0x64c26e in js::frontend::Parser<js::frontend::FullParseHandler>::statementListItem(js::frontend::YieldHandling, bool) js/src/frontend/Parser.cpp:7021:16
    #5 0x631bff in js::frontend::Parser<js::frontend::FullParseHandler>::statementList(js::frontend::YieldHandling) js/src/frontend/Parser.cpp:3814:21
    #6 0x65a359 in js::frontend::Parser<js::frontend::FullParseHandler>::functionBody(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::FunctionSyntaxKind, js::frontend::Parser<js::frontend::FullParseHandler>::FunctionBodyType) js/src/frontend/Parser.cpp:2394:14
    #7 0x65536d in js::frontend::Parser<js::frontend::FullParseHandler>::functionFormalParametersAndBody(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::ParseNode*, js::frontend::FunctionSyntaxKind) js/src/frontend/Parser.cpp:3472:17
    #8 0x62730c in js::frontend::Parser<js::frontend::FullParseHandler>::standaloneLazyFunction(JS::Handle<JSFunction*>, bool, js::GeneratorKind, js::FunctionAsyncKind) js/src/frontend/Parser.cpp:3388:10
    #9 0x1481a54 in js::frontend::CompileLazyFunction(JSContext*, JS::Handle<js::LazyScript*>, char16_t const*, unsigned long) js/src/frontend/BytecodeCompiler.cpp:650:21
    #10 0x10b966c in JSFunction::createScriptForLazilyInterpretedFunction(JSContext*, JS::Handle<JSFunction*>) js/src/jsfun.cpp:1540:14
    #11 0x130a564 in JSFunction::getOrCreateScript(JSContext*) js/src/jsfun.h:407:18
    #12 0x130a564 in EnsureFunctionHasScript(JSContext*, JS::Handle<JSFunction*>) js/src/vm/Debugger.cpp:225
    #13 0x130925b in js::Debugger::wrapDebuggeeObject(JSContext*, JS::Handle<JSObject*>, JS::MutableHandle<js::DebuggerObject*>) js/src/vm/Debugger.cpp:1165:14
    #14 0x130306f in js::Debugger::wrapDebuggeeValue(JSContext*, JS::MutableHandle<JS::Value>) js/src/vm/Debugger.cpp:1121:14
    #15 0x135620c in DebuggerGenericEval(JSContext*, mozilla::Range<char16_t const>, JS::Handle<JSObject*>, js::EvalOptions const&, JSTrapStatus&, JS::MutableHandle<JS::Value>, js::Debugger*, JS::Handle<JSObject*>, js::ScriptFrameIter*) js/src/vm/Debugger.cpp:7578:12
    #16 0x135358a in js::DebuggerFrame::eval(JSContext*, JS::Handle<js::DebuggerFrame*>, mozilla::Range<char16_t const>, JS::Handle<JSObject*>, js::EvalOptions const&, JSTrapStatus&, JS::MutableHandle<JS::Value>) js/src/vm/Debugger.cpp:7597:12
    #17 0x135aae8 in js::DebuggerFrame::evalMethod(JSContext*, unsigned int, JS::Value*) js/src/vm/Debugger.cpp:8176:10
    #18 0x156f6b4 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:239:15
    #19 0x156f6b4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:446
    #20 0x15504dc in js::CallFromStack(JSContext*, JS::CallArgs const&) js/src/vm/Interpreter.cpp:509:12
    #21 0x15504dc in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:2922
    #22 0x1533f73 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:404:12
    #23 0x156feb7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:476:15
    #24 0x1570932 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:522:10
    #25 0x1311252 in js::Call(JSContext*, JS::Handle<JS::Value>, JSObject*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.h:114:12
    #26 0x1311252 in js::Debugger::fireDebuggerStatement(JSContext*, JS::MutableHandle<JS::Value>) js/src/vm/Debugger.cpp:1732
    #27 0x1305ba5 in js::Debugger::slowPathOnDebuggerStatement(JSContext*, js::AbstractFramePtr)::$_5::operator()(js::Debugger*) const js/src/vm/Debugger.cpp:983:20
    #28 0x1305ba5 in JSTrapStatus js::Debugger::dispatchHook<js::Debugger::slowPathOnDebuggerStatement(JSContext*, js::AbstractFramePtr)::$_4, js::Debugger::slowPathOnDebuggerStatement(JSContext*, js::AbstractFramePtr)::$_5>(JSContext*, js::Debugger::slowPathOnDebuggerStatement(JSContext*, js::AbstractFramePtr)::$_4, js::Debugger::slowPathOnDebuggerStatement(JSContext*, js::AbstractFramePtr)::$_5) js/src/vm/Debugger.cpp:1873
    #29 0x1305ba5 in js::Debugger::slowPathOnDebuggerStatement(JSContext*, js::AbstractFramePtr) js/src/vm/Debugger.cpp:979
    #30 0x1540fb6 in js::Debugger::onDebuggerStatement(JSContext*, js::AbstractFramePtr) js/src/vm/Debugger-inl.h:58:12
    #31 0x1540fb6 in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3753
[....]
    #46 0x7f27c4f8fa5e  (<unknown module>)

SUMMARY: AddressSanitizer: heap-buffer-overflow js/src/frontend/FullParseHandler.h:921:16 in js::frontend::FullParseHandler::nextLazyClosedOverBinding()
Shadow bytes around the buggy address:
  0x0c047fffb940: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fd
=>0x0c047fffb950: fa fa fd fa fa fa fd fa fa fa fd fa fa fa 00[fa]
  0x0c047fffb960: fa fa 07 fa fa fa 00 fa fa fa 00 fa fa fa fd fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd


This seems related to the Debugger, so not marking s-s for now.

Comment 1

[Jan de Mooij [:jandem]]

(In reply to Christian Holler (:decoder) from comment #0)
> This seems related to the Debugger, so not marking s-s for now.

It's in the parser so I'm not sure it's Debugger-only.

Comment 2

[Jan de Mooij [:jandem]]

With a debug build, I get:

Assertion failure: lazyClosedOverBindingIndex < lazyOuterFunction()->numClosedOverBindings(), at FullParseHandler.h:920

Comment 3

[Fuzzing Team]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20160722042525" and the hash "fbaa8f469f5a80230e567b7517e0aa0a83ccaf65".
The "bad" changeset has the timestamp "20161108041535" and the hash "bd9dc9379305055245e0751095b6e6bdeb214b32".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fbaa8f469f5a80230e567b7517e0aa0a83ccaf65&tochange=bd9dc9379305055245e0751095b6e6bdeb214b32

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9a91fa1603c4
user:        Mariusz Kierski
date:        Sun Aug 28 20:42:39 2016 +0900
summary:     Bug 1185106 - Part 1: Add AsyncFunction flag in FunctionBox, JSScript, and LazyScript. r=efaust,till

Mariusz and Eric are no longer actively around, so setting needinfo? from Jan as a start.

Comment 5

[Till Schneidereit [:till]]

Forwarding needinfo to arai who drove the async/await support in.

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Fix NumClosedOverBindingsLimit value.

NumClosedOverBindingsLimit should've been updated along the number of bits for PackedView.numClosedOverBindings.

Fixed the NumClosedOverBindingsLimit value, and also added comments for NumClosedOverBindingsLimit and NumInnerFunctionsLimit, in PackedView members.

Comment 7

[Till Schneidereit [:till]]

Comment on attachment 8808863 [details] [diff] [review]
Fix NumClosedOverBindingsLimit value.

Review of attachment 8808863 [details] [diff] [review]:
-----------------------------------------------------------------

Is a static assert possible for this? If so, r=me with that added. Otherwise, r=me without :)

Comment 8

[Shu-yu Guo [:shu]]

TIL apparently you can use a constant expression on the right side of a bitfield :

e.g., we can do something like

static const uint32_t ClosedOverBindingsBits = 21;

struct PackedView {
  ...
  uint32_t numClosedOverBindings : ClosedOverBindingsBits;
  ...
};

static const uint32_t NumClosedOverBindingsLimit = 1 << ClosedOverBindingsBits;

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Part 1.1: Use class static const for bitfield size.

Comment 10

[Tooru Fujisawa [:arai]]

this is from bug 1185106, that is nightly-only.
I'll land Part 1 first.

Comment 11

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4559268d6068279ae6854d87219a31825ee9c501
Bug 1315943 - Part 1: Fix NumClosedOverBindingsLimit value. r=till

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4559268d6068

Comment 13

[Fuzzing Team]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 336759fad462).

Comment 14

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9afc1d0229c76f3876972638bbe398a91385e241
Bug 1315943 - Part 1.1: Use class static const for bitfield size. r=shu

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9afc1d0229c7

Comment 16

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.