Closed Bug 1936454 (CVE-2025-1011) Opened 1 year ago Closed 1 year ago

AddressSanitizer: TRAP on unknown address in js::wasm::Instance::callExport

Categories

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

defect

Tracking

()

RESOLVED FIXED
135 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox-esr128 135+ fixed
firefox133 --- wontfix
firefox134 --- wontfix
firefox135 + fixed

People

(Reporter: eternalsakuraalpha, Assigned: bvisness)

References

(Regression)

Details

(Keywords: regression, reporter-external, sec-moderate, Whiteboard: [client-bounty-form][adv-main135+][adv-ESR128.7+])

Attachments

(5 files, 1 obsolete file)

Attached file poc.js

Reproduce

  1. Download linux-asan-debug spidermonkey
  2. run poc.js

ASAN LOG

/home/sakura/.cache/autobisect/builds/js-m-c-linux-asan-debug-b16c09f16ef5/dist/bin/js poc.js
AddressSanitizer:DEADLYSIGNAL
=================================================================
==1915891==ERROR: AddressSanitizer: TRAP on unknown address 0x000000000000 (pc 0x1ef308b5010d bp 0x7ffe78c211d0 sp 0x7ffe78c21150 T0)
/home/sakura/.cache/autobisect/builds/js-m-c-linux-asan-debug-b16c09f16ef5/dist/bin/llvm-symbolizer: error: '[anon:js-executable-memory]': No such file or directory
    #0 0x1ef308b5010d  ([anon:js-executable-memory]+0x2010d)
    #1 0x1ef308b50b3e  ([anon:js-executable-memory]+0x20b3e)
    #2 0x557cdc9c01df in js::wasm::Instance::callExport(JSContext*, unsigned int, JS::CallArgs const&, js::wasm::CoercionLevel) /builds/worker/checkouts/gecko/js/src/wasm/WasmInstance.cpp:3695:10
    #3 0x557cdc9be4c3 in WasmCall(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/js/src/wasm/WasmInstance.cpp:3479:19
    #4 0x557cd986c122 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:532:13
    #5 0x557cd986ada6 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:628:12
    #6 0x557cd988cfcc in js::CallFromStack(JSContext*, JS::CallArgs const&, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:700:10
    #7 0x557cd988cfcc in js::Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:3338:16
    #8 0x557cd98690a5 in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:502:13
    #9 0x557cd98714e4 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:893:13
    #10 0x557cd9872179 in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:926:10
    #11 0x557cd9bb9b0a in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/CompilationAndEvaluation.cpp:601:10
    #12 0x557cd9bb9f8f in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) /builds/worker/checkouts/gecko/js/src/vm/CompilationAndEvaluation.cpp:625:10
    #13 0x557cd95ddbae in RunFile(JSContext*, char const*, _IO_FILE*, CompileUtf8, bool, bool) /builds/worker/checkouts/gecko/js/src/shell/js.cpp:1307:10
    #14 0x557cd95dc555 in Process(JSContext*, char const*, bool, FileKind) /builds/worker/checkouts/gecko/js/src/shell/js.cpp
    #15 0x557cd9501512 in ProcessArgs(JSContext*, js::cli::OptionParser*) /builds/worker/checkouts/gecko/js/src/shell/js.cpp:11741:10
    #16 0x557cd9501512 in Shell(JSContext*, js::cli::OptionParser*) /builds/worker/checkouts/gecko/js/src/shell/js.cpp:11993:12
    #17 0x557cd94ef02e in main /builds/worker/checkouts/gecko/js/src/shell/js.cpp:12408:12
    #18 0x7fc795a29d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

==1915891==Register values:
rax = 0x000033fc75300080  rbx = 0x00002241d35710e0  rcx = 0x0000507000000480  rdx = 0x0000507000004310  
rdi = 0x0000000000000000  rsi = 0x0000000000000000  rbp = 0x00007ffe78c211d0  rsp = 0x00007ffe78c21150  
 r8 = 0x00000fffcf1842dd   r9 = 0x00007ffe78c21700  r10 = 0x00007ffe78c21428  r11 = 0x00000000ffffffff  
r12 = 0x00007ffe78c21248  r13 = 0x00007ffe78c21428  r14 = 0x0000519000086b80  r15 = 0x00007fc58b3ab000  
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: TRAP ([anon:js-executable-memory]+0x2010d) 
==1915891==ABORTING
Flags: sec-bounty?
Group: firefox-core-security → core-security
Component: Security → JavaScript: WebAssembly
Product: Firefox → Core
Group: core-security → javascript-core-security
Attached file poc.js (obsolete) —

Here's the same test using the text format. I wasn't able to repro on OSX debug ASAN locally, going to switch to my linux machine.

Attachment #9443028 - Attachment mime type: application/x-javascript → text/plain
Attached file poc.js

Minimized.

Attachment #9443028 - Attachment is obsolete: true
    struct.new_default 1
    local.set 0

    loop
      local.get 0
      ref.cast i31ref
      i31.get_s
      i32.const 0
      i32.store8

      br 0
    end

The issue here is that we're somehow hoisting the i31.get_s operation to happen before the cast operation (which should always fail). I do not know why that is happening.

Priority: -- → P1

It seems to simply be that MWasmI31RefGet generated by i31.get_s is marked as movable, and gets hoisted above both the ref.cast and the MWasmTrapIfNull that is also generated. This is obviously wrong, but since the MWasmI31RefGet doesn't depend on either of those two instructions, and it's movable, LICM will move it up.

A quick fix is to make MWasmI31RefGet immovable like other ref accesses. A better fix would be to change how casting works, and how MIR is generated for i31.get_s, to ensure that we have explicit dependencies between these nodes in MIR.

As for security impact, it seems that i31.get_s and i31.get_u are the only ref-loading operations that are marked as movable. Struct/array loads and stores are not movable, table.set is not movable, call_indirect and call_ref are not movable, and other ref operations do nothing but treat it as an opaque value. This bug could possibly allow you to load a pointer value from outside the sandbox, but I don't believe it would allow you to actually load or call anything from the host.

It's also worth noting that I haven't yet come up with a way to actually do anything with the i31 value that was prematurely loaded. WebAssembly validation ensures that we must ref.cast before actually performing an operation such as a load, store, or call, and such operations are not also movable. So while the load can incorrectly occur too early, the cast will still fail, correctly, before the bad value can be exfiltrated.

The following factors all make this bug less severe, in my opinion:

  • It's somewhat difficult to repro even with the knowledge that i31.get should not be movable
  • There's no clear way to exfiltrate the data because:
    • Exploiting this requires you to use anyref and ref.cast to prevent validation from failing on i31.get
    • The ref.cast will still run correctly and trap before the result of i31.get can be used
  • Loading a host pointer value as an integer still does not directly allow you to load or call anything from the host

So, I think we would be ok to just ship the easy fix, even though removing movable: true makes the nature of the bug somewhat obvious.

Assignee: nobody → bvisness
Status: NEW → ASSIGNED

This was added by i31ref support which was shipped in Fx120 through bug 1845373.

Keywords: regression
Regressed by: 1692065

Set release status flags based on info from the regressing bug 1692065

Severity: -- → S2
Keywords: sec-moderate
Attachment #9444088 - Flags: approval-mozilla-beta?

beta Uplift Approval Request

  • User impact if declined: A bug with bad codegen could possibly be exploited in ways we currently cannot imagine.
  • Code covered by automated testing: yes
  • Fix verified in Nightly: no
  • Needs manual QE test: no
  • Steps to reproduce for manual QE testing: -
  • Risk associated with taking this patch: Low to none
  • Explanation of risk level: Because this patch makes an instruction immovable, it makes the code more predictable.
  • String changes made/needed: n/a
  • Is Android affected?: yes
Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 135 Branch
Flags: sec-bounty? → sec-bounty+
Attachment #9444088 - Flags: approval-mozilla-esr128?

Comment on attachment 9444088 [details]
Bug 1936454: Make i31.get immovable. r=rhunt

We're past the point of taking security uplifts for 134 this cycle, but I've repurposed this revision for the ESR128 uplift next cycle that we'll need to do still.

Attachment #9444088 - Flags: approval-mozilla-beta?
QA Whiteboard: [post-critsmash-triage]
Flags: qe-verify-
Attachment #9444088 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+
Whiteboard: [client-bounty-form] → [client-bounty-form][adv-main135+]
Whiteboard: [client-bounty-form][adv-main135+] → [client-bounty-form][adv-main135+][adv-ESR128.7+]
Attached file advisory.txt
Alias: CVE-2025-1011
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: