Closed Bug 1668589 Opened 4 years ago Closed 4 years ago

UndefinedBehaviorSanitizer: mozilla/Maybe.h:295:46: runtime error: load of value 95869805, which is not a valid value for type 'typename std::remove_reference<ABIFunctionType &>::type'

Categories

(Core :: JavaScript: WebAssembly, defect, P3)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
83 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox82 --- wontfix
firefox83 --- fixed

People

(Reporter: decoder, Assigned: lth)

References

(Blocks 1 open bug)

Details

(5 keywords, Whiteboard: [bugmon:update,bisect,confirmed][post-critsmash-triage][adv-main83-][adv-main83-])

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 20201001-ba35799faec2 (asan-opt build, run with --fuzzing-safe --no-threads):

(function(stdlib, foreign) {
    "use asm";
    var ff = foreign.ff;
    function f(i0, d1) {
        i0 = i0 | 0;
        d1 = +d1;
        + ff(.0, 3., -0, -0, -0, 3., .8, .0)
    }
    return f
})(this, {
    ff: Int8Array
}, new ArrayBuffer())()

Backtrace:

dist/include/mozilla/Maybe.h:295:46: runtime error: load of value 95869805, which is not a valid value for type 'typename std::remove_reference<ABIFunctionType &>::type' (aka 'js::jit::ABIFunctionType')
error: address range table at offset 0x1960 has an invalid tuple (length = 0) at offset 0x1ea0
    #0 0x555ce2941497 in Union dist/include/mozilla/Maybe.h:295:46
    #1 0x555ce2941497 in MaybeStorage dist/include/mozilla/Maybe.h:309:9
    #2 0x555ce2941497 in Maybe<js::jit::ABIFunctionType> dist/include/mozilla/Maybe.h:387:9
    #3 0x555ce2941497 in Some<js::jit::ABIFunctionType, js::jit::ABIFunctionType> dist/include/mozilla/Maybe.h:831:10
    #4 0x555ce2941497 in ToBuiltinABIFunctionType js/src/wasm/WasmBuiltins.cpp:1499:10
    #5 0x555ce2941497 in js::wasm::MaybeGetBuiltinThunk(JSFunction*, js::wasm::FuncType const&) js/src/wasm/WasmBuiltins.cpp:1510:36
    #6 0x555ce2a17d5b in js::wasm::Instance::init(JSContext*, JS::GCVector<JSFunction*, 0ul, js::SystemAllocPolicy> const&, JS::GCVector<js::wasm::Val, 0ul, js::SystemAllocPolicy> const&, JS::GCVector<js::WasmGlobalObject*, 0ul, js::SystemAllocPolicy> const&, mozilla::Vector<js::wasm::SerializableRefPtr<js::wasm::DataSegment const>, 0ul, js::SystemAllocPolicy> const&, mozilla::Vector<js::wasm::SerializableRefPtr<js::wasm::ElemSegment const>, 0ul, js::SystemAllocPolicy> const&) js/src/wasm/WasmInstance.cpp:1463:30
    #7 0x555ce2a96e81 in js::WasmInstanceObject::create(JSContext*, RefPtr<js::wasm::Code const>, mozilla::Vector<js::wasm::SerializableRefPtr<js::wasm::DataSegment const>, 0ul, js::SystemAllocPolicy> const&, mozilla::Vector<js::wasm::SerializableRefPtr<js::wasm::ElemSegment const>, 0ul, js::SystemAllocPolicy> const&, mozilla::UniquePtr<js::wasm::TlsData, js::wasm::TlsDataDeleter>, JS::Handle<js::WasmMemoryObject*>, mozilla::Vector<RefPtr<js::wasm::Table>, 0ul, js::SystemAllocPolicy>&&, JS::GCVector<js::HeapPtr<js::StructTypeDescr*>, 0ul, js::SystemAllocPolicy>&&, JS::GCVector<JSFunction*, 0ul, js::SystemAllocPolicy> const&, mozilla::Vector<js::wasm::GlobalDesc, 0ul, js::SystemAllocPolicy> const&, JS::GCVector<js::wasm::Val, 0ul, js::SystemAllocPolicy> const&, JS::GCVector<js::WasmGlobalObject*, 0ul, js::SystemAllocPolicy> const&, JS::Handle<JSObject*>, mozilla::UniquePtr<js::wasm::DebugState, JS::DeletePolicy<js::wasm::DebugState> >) js/src/wasm/WasmJS.cpp:1818:18
    #8 0x555ce2a8777c in js::wasm::Module::instantiate(JSContext*, js::wasm::ImportValues&, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) const js/src/wasm/WasmModule.cpp:1410:16
    #9 0x555ce28c5f99 in TryInstantiate js/src/wasm/AsmJS.cpp:6860:15
    #10 0x555ce28c5f99 in js::InstantiateAsmJS(JSContext*, unsigned int, JS::Value*) js/src/wasm/AsmJS.cpp:6959:8
    #11 0x555ce1222a04 in CallJSNative js/src/vm/Interpreter.cpp:508:13
    [...]

I'm marking this s-s because it looks like a potential type confusion or uninitialized memory.

Attached file Testcase (deleted) —
Blocks: ubsan

Bugmon Analysis:
Unable to reproduce bug using the following builds:

mozilla-central 20201001094020-ba35799faec2
mozilla-central 20201001094020-ba35799faec2
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Keywords: bugmon
Whiteboard: [bugmon:update,bisect] → [bugmon:update,bisect,confirmed]

OK, maybe a stupid question, how do I build with asan? I tried --enable-address-sanitizer and I get a bunch of duplicate operator definition link errors. I thus tried --disable-jemalloc and I get some missing function definition link errors.

Flags: needinfo?(choller)

UTR on current m-c:

$ dist/bin/js --fuzzing-safe --no-threads ~/moz/t.js
/home/lhansen/moz/t.js:7:2 TypeError: calling a builtin typed array constructor without new is forbidden
Stack:
  f@/home/lhansen/moz/t.js:7:1
  @/home/lhansen/moz/t.js:12:22

Configuration:

../configure --disable-jemalloc --enable-address-sanitizer --enable-gczeal --disable-debug --enable-optimize="-O2 -g" --without-intl-api

Altogether a curious error. ABIFunctionType is an enum, and abiType is a uint32_t. We're basically constructing a value that will be cast to that enum. If there's an error here what the error is saying is that the value we're constructing is not a member of that enum. The value is 0x5b6db6d aka 101 101 101 101 101 101 101 101 101 where the last 101 is the return type and the others are argument types, https://searchfox.org/mozilla-central/source/js/src/jit/IonTypes.h#906. 101 is Float64. This is thus (f64, f64, f64, f64, f64, f64, f64, f64) -> f64, this corresponds well to the call in the test case. Now clearly this value is not part of the enum yet we're casting it as such. According to https://en.cppreference.com/w/cpp/language/enum this is not well-defined if the type of the enum is not explicitly stated (and it is not), indeed it is UB since C++17 to do this. It would be not-UB if the enum were declared as ": uint32_t", IIUC.

Ah, and the title of the bug says this is ubsan but comment 0 says asan, hence I've been testing with asan. But it makes a lot more sense if it is ubsan that reacted. I'll make a small change to properly tag the enum.

See bug for further info.

Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Severity: -- → S3
Priority: -- → P3

Going to downgrade this to sec-other, there's no reason to believe that that enum would not have an int representation on reasonable compilers, ie, this is not a bug in the real world.

Keywords: sec-highsec-other
Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch
Flags: needinfo?(choller)
Flags: qe-verify-
Whiteboard: [bugmon:update,bisect,confirmed] → [bugmon:update,bisect,confirmed][post-critsmash-triage]
Whiteboard: [bugmon:update,bisect,confirmed][post-critsmash-triage] → [bugmon:update,bisect,confirmed][post-critsmash-triage][adv-main83-]
Whiteboard: [bugmon:update,bisect,confirmed][post-critsmash-triage][adv-main83-] → [bugmon:update,bisect,confirmed][post-critsmash-triage][adv-main83-][adv-main83-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: