Closed Bug 1472634 Opened 6 years ago Closed 6 years ago

Assertion failure: aIndex < mLength, at dist/include/mozilla/Vector.h:552 with wasm


(Core :: JavaScript: WebAssembly, defect)

Not set



Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- disabled
firefox63 --- fixed


(Reporter: decoder, Assigned: bbouvier)



(4 keywords)


(3 files)

The attached binary WebAssembly testcase crashes on mozilla-inbound revision c65f021935f1 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2 --without-intl-api --enable-debug). To reproduce, you can run the following code in the JS shell:

var data = os.file.readFile(file, 'binary');
new WebAssembly.Instance(new WebAssembly.Module(data.buffer));


==22955==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000026406bc bp 0x7ffce62d0ae0 sp 0x7ffce62d0ab0 T0)
==22955==The signal is caused by a WRITE memory access.
==22955==Hint: address points to the zero page.
    #0 0x26406bb in mozilla::Vector<js::wasm::TypeDef, 0ul, js::SystemAllocPolicy>::operator[](unsigned long) const dist/include/mozilla/Vector.h:552:5
    #1 0x26406bb in js::wasm::OpIter<js::wasm::BaseCompiler::BaseCompilePolicy>::IsPrefixOf(js::wasm::StackType, js::wasm::StackType) js/src/wasm/WasmOpIter.h:712
    #2 0x26400b0 in js::wasm::OpIter<js::wasm::BaseCompiler::BaseCompilePolicy>::IsSubtypeOf(js::wasm::StackType, js::wasm::StackType) js/src/wasm/WasmOpIter.h:721:70
    #3 0x26481b7 in js::wasm::OpIter<js::wasm::BaseCompiler::BaseCompilePolicy>::Join(js::wasm::StackType, js::wasm::StackType, js::wasm::StackType*) js/src/wasm/WasmOpIter.h:773:13
    #4 0x260280a in js::wasm::OpIter<js::wasm::BaseCompiler::BaseCompilePolicy>::readSelect(js::wasm::StackType*, mozilla::Nothing*, mozilla::Nothing*, mozilla::Nothing*) js/src/wasm/WasmOpIter.h:1549:10
    #5 0x256f532 in js::wasm::BaseCompiler::emitSelect() js/src/wasm/WasmBaselineCompile.cpp:8654:16
    #6 0x257d2ef in js::wasm::BaseCompiler::emitBody() js/src/wasm/WasmBaselineCompile.cpp:9421:13
    #7 0x2593d86 in js::wasm::BaseCompiler::emitFunction() js/src/wasm/WasmBaselineCompile.cpp:10102:10
    #8 0x2593d86 in js::wasm::BaselineCompileFunctions(js::wasm::ModuleEnvironment const&, js::LifoAlloc&, mozilla::Vector<js::wasm::FuncCompileInput, 8ul, js::SystemAllocPolicy> const&, js::wasm::CompiledCode*, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/wasm/WasmBaselineCompile.cpp:10255
    #9 0x268d177 in ExecuteCompileTask(js::wasm::CompileTask*, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/wasm/WasmGenerator.cpp:629:14
    #10 0x268de69 in js::wasm::ModuleGenerator::launchBatchCompile() js/src/wasm/WasmGenerator.cpp:698:14
    #11 0x268f007 in js::wasm::ModuleGenerator::finishFuncDefs() js/src/wasm/WasmGenerator.cpp:774:26
    #12 0x25be72b in bool DecodeCodeSection<js::wasm::Decoder>(js::wasm::ModuleEnvironment const&, js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/wasm/WasmCompile.cpp:84:15
    #13 0x25bce68 in js::wasm::CompileBuffer(js::wasm::CompileArgs const&, js::wasm::ShareableBytes const&, mozilla::UniquePtr<char [], JS::FreePolicy>*, mozilla::Vector<mozilla::UniquePtr<char [], JS::FreePolicy>, 0ul, js::SystemAllocPolicy>*) js/src/wasm/WasmCompile.cpp:444:10
    #14 0x26f5ce8 in js::wasm::Eval(JSContext*, JS::Handle<js::TypedArrayObject*>, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) js/src/wasm/WasmJS.cpp:346:27
    #15 0x5d9040 in WasmLoop(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:6754:14

Marking s-s because this is a range assertion.
Attached file Testcase
Caused by bug 1459900. When creating a nullref, we have to specify which type it is, for validation purposes; the referenced struct type is non existent here.

It's a Nightly only bug, because it's controlled by the --wasm-gc flag. Christian, can you confirm this flag was used during testing? If it's omitted, I can locally see a "CompileError" with "bad type", which is expected (because without --wasm-gc, the new ref types are not known to the compiler).
Depends on: 1459900
Flags: needinfo?(choller)
Assignee: nobody → bbouvier
Confirmed, this was with --wasm-gc. The pool that was testing with that flag was still running. I suspect that the other bug I reported might as well be from that pool. I've disabled it now until I hear back from you and we are clear to test this again.
Flags: needinfo?(choller)
Just moves binary testing code around in the lib file, so we can have a binary test case under wasm/gc.
Attachment #8989955 - Flags: review?(jseward)
Attached patch 2.fix.patchSplinter Review
The fix in itself. Just check that when creating a null ref, the referenced type actually exists.
Attachment #8989956 - Flags: review?(jseward)
Attachment #8989955 - Flags: review?(jseward) → review+
Comment on attachment 8989956 [details] [diff] [review]

Review of attachment 8989956 [details] [diff] [review]:

::: js/src/wasm/WasmOpIter.h
@@ +1768,5 @@
>      uint32_t refTypeIndex;
>      if (!d_.readValType(&code, &refTypeIndex))
>          return fail("unknown nullref type");
>      if (code == uint8_t(TypeCode::Ref)) {
> +        if (refTypeIndex > MaxTypes || refTypeIndex >= env_.types.length())

Quick sanity check: is "> MaxTypes" right here?  Should it be ">= MaxTypes"?
Attachment #8989956 - Flags: review?(jseward) → review+
(In reply to Julian Seward [:jseward] from comment #6)
> Quick sanity check: is "> MaxTypes" right here?  Should it be ">= MaxTypes"?

Particularly given that the patch in bug appears to use "< MaxTypes"
as an in-range check.
Good catch, thanks!
Group: javascript-core-security → core-security-release
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Opening up, since this is nightly-only and protected behind --wasm-gc.
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.