Closed Bug 1597200 Opened 10 months ago Closed 10 months ago

Assertion failure: srcOffset >= bytes, at js/src/wasm/WasmBaselineCompile.cpp:1810

Categories

(Core :: Javascript: WebAssembly, defect, P1)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- unaffected
firefox71 --- unaffected
firefox72 --- fixed

People

(Reporter: decoder, Assigned: wingo)

Details

(4 keywords, Whiteboard: [post-critsmash-triage])

Attachments

(2 files)

The attached testcase crashes on mozilla-central revision 72c52c0101cf (build with --enable-valgrind --enable-gczeal --enable-tests --enable-fuzzing --enable-debug --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2, run with --no-threads --wasm-compiler=baseline test.js).

Backtrace:

==28810==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x55d11cb618c0 bp 0x7ffe3bc0f370 sp 0x7ffe3bc0f1e0 T0)
==28810==The signal is caused by a WRITE memory access.
==28810==Hint: address points to the zero page.
    #0 0x55d11cb618bf in js::wasm::BaseStackFrame::shuffleStackResultsTowardFP(unsigned int, unsigned int, unsigned int, js::jit::Register) js/src/wasm/WasmBaselineCompile.cpp:1809:5
    #1 0x55d11cb5d6b9 in js::wasm::BaseCompiler::popStackResults(js::wasm::ABIResultIter&, js::wasm::StackHeight) js/src/wasm/WasmBaselineCompile.cpp:4074:12
    #2 0x55d11ca34409 in js::wasm::BaseCompiler::popBlockResults(js::wasm::ResultType, js::wasm::StackHeight, js::wasm::BaseCompiler::ContinuationKind) js/src/wasm/WasmBaselineCompile.cpp:4161:9
    #3 0x55d11ca3d333 in js::wasm::BaseCompiler::emitBr() js/src/wasm/WasmBaselineCompile.cpp:8848:3
    #4 0x55d11ca8a4b0 in js::wasm::BaseCompiler::emitBody() js/src/wasm/WasmBaselineCompile.cpp:11389:9
    #5 0x55d11caf9f03 in js::wasm::BaseCompiler::emitFunction() js/src/wasm/WasmBaselineCompile.cpp:12278:8
    #6 0x55d11caf9f03 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:12443
    #7 0x55d11cce853e in ExecuteCompileTask(js::wasm::CompileTask*, mozilla::UniquePtr<char [], JS::FreePolicy>*) js/src/wasm/WasmGenerator.cpp:743:12
    #8 0x55d11ccec124 in js::wasm::ModuleGenerator::locallyCompileCurrentTask() js/src/wasm/WasmGenerator.cpp:776:8
    #9 0x55d11ccec124 in js::wasm::ModuleGenerator::finishFuncDefs() js/src/wasm/WasmGenerator.cpp:914
    #10 0x55d11cb37922 in bool DecodeCodeSection<js::wasm::Decoder>(js::wasm::ModuleEnvironment const&, js::wasm::Decoder&, js::wasm::ModuleGenerator&) js/src/wasm/WasmCompile.cpp:570:13
    #11 0x55d11cb368db 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::OptimizedEncodingListener*) js/src/wasm/WasmCompile.cpp:593:8
    #12 0x55d11ce29eb8 in js::WasmModuleObject::construct(JSContext*, unsigned int, JS::Value*) js/src/wasm/WasmJS.cpp:1180:7
    [...]

This is similar to bug 1595466 but seems to reproduce even after the fix for that.

Attached file Testcase

The rev in comment 0 is older because I forgot to update the build on the triage machine, but I just also reproduced this on d6844fe546ad.

Maybe another fallout from multi-value?

Flags: needinfo?(wingo)
Priority: -- → P1

Thanks, will look.

Assignee: nobody → wingo
Flags: needinfo?(wingo)

Amusingly, this needed a fix to wabt to decode: https://github.com/WebAssembly/wabt/pull/1231

For the record:

$ ~/src/wabt/out/gcc/Debug/wasm2wat --enable-threads --enable-sign-extension --enable-multi-value --no-check test.wasm
(module
  (type (;0;) (func (param i32 i32 i32) (result i32)))
  (func $main (type 0) (param i32 i32 i32) (result i32)
    local.get 0
    i32.const -63
    br_if 0 (;@0;)
    loop (result i32)  ;; label = @1
      local.get 0
      i32.eqz
      i32.eqz
      i32.const -8
      i32.popcnt
      i32.div_s
      i32.const -23
      i32.div_s
      i32.const 59
      i32.const -2
      i32.popcnt
      i32.popcnt
      loop (param i32 i32 i32) (result i32)  ;; label = @2
        atomic.fence
        atomic.fence
        atomic.fence
        atomic.fence
        i32.const -23
        i32.div_s
        i32.const -63
        i32.eqz
        i32.eqz
        br 0 (;@2;)
      end
      unreachable
      return
      unreachable
      atomic.fence
      i32.const -8
      i32.popcnt
      i32.div_s
      i32.const 0
      unreachable
      i32.popcnt
      return
      i32.extend8_s
      unreachable
      i32.popcnt
      atomic.fence
      unreachable
    end)
  (export "main" (func $main)))

Minimal repro:

new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(`
(module
  (func (export "main") (result i32)
    (i32.const 1)
    (i32.const 2)
    (i32.const 3)
    (loop (param i32 i32 i32)
      (i32.popcnt)
      (i32.const -63)
      (br 0))
    (unreachable)))`)));

In a sadly similar bogosity as in bug 1595466, the multi-value stack
value shuffling code was confused about "stack height" (distance from
FP), "stack offset" (distance from SP), and whether the bytes to be
copied were towards the FP or the SP.

I am a bit speechless that this code had bugs that were this obvious :/ I will try to produce a test case that can be called.

sec-moderate because there might be some risk here of exposing heap addresses if we can confuse a pointer with an integer.

Keywords: sec-moderate

For the record, this bug is a shell-only Nightly bug -- i.e., not the browser.

(In reply to Andy Wingo [:wingo] from comment #11)

For the record, this bug is a shell-only Nightly bug -- i.e., not the browser.

Yeah. See https://bugzilla.mozilla.org/show_bug.cgi?id=1595466#c12 et seq for reasons for sec-moderate. But this is fine to just land, for sure.

Group: javascript-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.