PatchGap
WebKit · WebKit › JSC Wasm

CVE-2024-54508

Type Confusion in WebKit (JSC Wasm)
Medium CVSS 7.5 webkit Type Confusion 🔧 Commit mapped

Overview

Medium
Severity
7.5
CVSS
No
Exploited ITW
Fixed
Fix Status
DescriptionProcessing maliciously crafted web content may lead to an unexpected process crash
ComponentWebKit
Sub-componentJSC Wasm
Bug ClassType Confusion
Tracker282180
Fix commit6146215d9220 (WebKit/WebKit)
CWECWE-125, CWE-787 (Out-of-bounds read, Out-of-bounds write)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CISA KEVNot listed
CreditedXiangwei Zhang of Tencent Security YUNDING LAB, linjy of HKUS3Lab and chluo of WHUSecLab, and an anonymous researcher
Disclosed2024-12-11

Background

BBQ JIT
JavaScriptCore’s baseline wasm optimizing tier with its own register allocator.
Register bindings
m_gprBindings/m_fprBindings map physical registers to the live values they currently hold.
consume / unbind
Releases a value’s register binding so the allocator no longer assumes that register holds it.
Unreachable region
Wasm code after an unconditional branch/throw that is still validated and compiled, with special control handling.

Root Cause Analysis

In JavaScriptCore’s Wasm BBQ JIT, register allocation tracks which GPRs/FPRs are bound to live stack values via m_gprBindings/m_fprBindings. When compiling exception-handling catch blocks, the register-binding state must be reset consistently on entry to the catch and when finishing an unreachable region, or the JIT’s model of which registers hold which values diverges from reality.

Pre-patch, addCatch and addCatchAll did not unbind the currently-bound registers on entry, while addEndToUnreachable open-coded a loop that consumed all bindings. That asymmetry meant compiling a catch (especially in an unreachable context, per the compile-unreachable-catch test) left stale register bindings, so subsequent codegen could treat a register as holding a value it no longer does — a JIT register-state inconsistency that miscompiles and crashes.

The fix factors the binding-clearing into unbindAllRegisters() (consuming every non-none GPR/FPR binding) and calls it at the start of addCatch and addCatchAll as well as reusing it in addEndToUnreachable, so all three paths reset register state identically.

The restored invariant is that entering a catch handler or an unreachable end clears the register bindings, keeping the BBQ JIT’s register model consistent.

Key insight
BBQ-JIT catch handlers didn’t clear register bindings on entry the way the unreachable-end path did, so stale bindings miscompiled the catch; unifying on unbindAllRegisters() restores a consistent register model.

Attack Path

  1. Craft wasm with unreachable catch Build a module whose function has an exception-handling catch/catch_all reached in an unreachable code region.
  2. Compile via BBQ JIT Instantiation compiles the function; addCatch/addCatchAll run without clearing prior register bindings.
  3. Diverge register state Stale GPR/FPR bindings persist into the catch codegen, so a register is assumed to hold a value it no longer holds.
  4. Miscompile / corrupt The resulting code uses the wrong register contents, crashing or corrupting memory in the WebContent process.

Impact Assessment

A BBQ-JIT register-state inconsistency compiling catch handlers (advisory class LogicError) that miscompiles code, reachable from crafted wasm in the WebContent process. A JIT register mismatch is a corruption-class primitive, though building a controlled result needs precise register shaping; the observable is a crash. Confined to WebContent; rated medium.

Changed Functions

FunctionChangeNotes
for
Source/JavaScriptCore/wasm/WasmBBQJIT.cpp
 modified
if
JSTests/wasm/stress/compile-unreachable-catch.js
 modified

Files Changed

  • Source/JavaScriptCore/wasm/WasmBBQJIT.cpp
  • Source/JavaScriptCore/wasm/WasmBBQJIT.h
  • JSTests/wasm/stress/compile-unreachable-catch.js

Audit Directions

  • Other control-entry binding resets
    grep WasmBBQJIT for control-flow entries (loop/block/try/delegate) that should unbindAllRegisters() but hand-roll or omit it.
  • Unreachable-region codegen
    Audit addEndToUnreachable and unreachable catch/delegate paths for register/stack-state assumptions.
  • consume() coverage
    Review addArray*/addStruct*/call paths for values whose registers are not consumed, leaving stale bindings.

Patch

view commit
diff --git a/Source/JavaScriptCore/wasm/WasmBBQJIT.cpp b/Source/JavaScriptCore/wasm/WasmBBQJIT.cpp
index 092590b9bdec..1e8c7e32a6f1 100644
--- a/Source/JavaScriptCore/wasm/WasmBBQJIT.cpp
+++ b/Source/JavaScriptCore/wasm/WasmBBQJIT.cpp
@@ -3463,6 +3463,7 @@ PartialResult WARN_UNUSED_RETURN BBQJIT::addCatch(unsigned exceptionIndex, const
 PartialResult WARN_UNUSED_RETURN BBQJIT::addCatchToUnreachable(unsigned exceptionIndex, const TypeDefinition& exceptionSignature, ControlType& data, ResultList& results)
 {
     m_usesExceptions = true;
+    unbindAllRegisters();
     ControlData dataCatch(*this, BlockType::Catch, data.signature(), data.enclosedHeight());
     dataCatch.setCatchKind(CatchKind::Catch);
     if (ControlData::isTry(data)) {
@@ -3507,6 +3508,7 @@ PartialResult WARN_UNUSED_RETURN BBQJIT::addCatchAll(Stack& expressionStack, Con
 PartialResult WARN_UNUSED_RETURN BBQJIT::addCatchAllToUnreachable(ControlType& data)
 {
     m_usesExceptions = true;
+    unbindAllRegisters();
     ControlData dataCatch(*this, BlockType::Catch, data.signature(), data.enclosedHeight());
     dataCatch.setCatchKind(CatchKind::CatchAll);
     if (ControlData::isTry(data)) {
@@ -3749,14 +3751,7 @@ PartialResult WARN_UNUSED_RETURN BBQJIT::addEndToUnreachable(ControlEntry& entry
             Type type = entryData.signature()->returnType(i);
             entry.enclosedExpressionStack.constructAndAppend(type, Value::fromTemp(type.kind, entryData.enclosedHeight() + entryData.implicitSlots() + i));
         }
-        for (const auto& binding : m_gprBindings) {
-            if (!binding.isNone())
-                consume(binding.toValue());
-        }
-        for (const auto& binding : m_fprBindings) {
-            if (!binding.isNone())
-                consume(binding.toValue());
-        }
+        unbindAllRegisters();
     } else {
         unsigned offset = stack.size() - returnCount;
         for (unsigned i = 0; i < returnCount; ++i)
@@ -4872,6 +4867,18 @@ void BBQJIT::unbind(Value value, Location loc)
         dataLogLn("BBQ\tUnbound value ", value, " from ", loc);
 }
 
+void BBQJIT::unbindAllRegisters()
+{
+    for (const auto& binding : m_gprBindings) {
+        if (!binding.isNone())
+            consume(binding.toValue());
+    }
+    for (const auto& binding : m_fprBindings) {
+        if (!binding.isNone())
+            consume(binding.toValue());
+    }
+}
+
 GPRReg BBQJIT::nextGPR()
 {
     auto next = m_gprSet.begin();
diff --git a/Source/JavaScriptCore/wasm/WasmBBQJIT.h b/Source/JavaScriptCore/wasm/WasmBBQJIT.h
index f33f42b4053e..ad1ced3e5c1d 100644
--- a/Source/JavaScriptCore/wasm/WasmBBQJIT.h
+++ b/Source/JavaScriptCore/wasm/WasmBBQJIT.h
@@ -1955,6 +1955,8 @@ private:
 
     void unbind(Value value, Location loc);
 
+    void unbindAllRegisters();
+
     template<typename Register>
     static Register fromJSCReg(Reg reg)
     {
diff --git a/JSTests/wasm/stress/compile-unreachable-catch.js b/JSTests/wasm/stress/compile-unreachable-catch.js
new file mode 100644
index 000000000000..c509346e9189
--- /dev/null
+++ b/JSTests/wasm/stress/compile-unreachable-catch.js
@@ -0,0 +1,77 @@
+import { instantiate } from "../wabt-wrapper.js";
+import * as assert from "../assert.js";
+
+const watGetGlobalsSameTempReturnCall = `
+(module
+  (tag $e (param f32))
+  (global $f0 (mut f32)(f32.const 2.0))
+  (global $i0 (mut i64)(i64.const 1))
+  (func (export "test") (local i64)
+    try
+      local.get 0
+      global.get $f0
+      return_call $bar
+    catch_all
+      global.get $i0
+      return_call $foo
+    end
+  )
+  (func $foo)
+  (func $bar)
+)
+`;
+
+const watGetGlobalCatchArgSameTempReturnCall = `
+(module
+  (tag $e (param i64))
+  (global $f0 (mut f32)(f32.const 2.0))
+  (global $i0 (mut i64)(i64.const 1))
+  (func (export "test") (result i64)
+    try (result i64)
+      global.get $f0
+      global.get $f0
+      return_call $bar
+    catch $e
+      global.get $i0
+      i64.add
+    end
+  )
+  (func $bar (param f32) (result i64) i64.const 3)
+)
+`;
+
+const watGetGlobalsSameTempUnreachable = `
+(module
+  (tag $e (param f32))
+  (global $f0 (mut f32)(f32.const 2.0))
+  (global $i0 (mut i64)(i64.const 1))
+  (func (export "test") (local i64)
+    try
+      local.get 0
+      global.get $f0
+      unreachable
+    catch_all
+      global.get $i0
+      return_call $foo
+    end
+  )
+  (func $foo)
+  (func $bar)
+)
+`;
+
+async function runOne(wat, isUnreachable) {
+    const instance = await instantiate(wat, {}, { exceptions: true, tail_call: true });
+    const {test} = instance.exports;
+    if (isUnreachable) {
+        assert.throws(() => {
+            test();
+        }, WebAssembly.RuntimeError, `Unreachable code`);
+    } else {
+        test();
+    }
+}
+
+assert.asyncTest(runOne(watGetGlobalsSameTempReturnCall, false));
+assert.asyncTest(runOne(watGetGlobalCatchArgSameTempReturnCall, false));
+assert.asyncTest(runOne(watGetGlobalsSameTempUnreachable, true));
\ No newline at end of file
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/JSTests/wasm/stress/compile-unreachable-catch.js b/JSTests/wasm/stress/compile-unreachable-catch.js
new file mode 100644
index 000000000000..c509346e9189
--- /dev/null
+++ b/JSTests/wasm/stress/compile-unreachable-catch.js
@@ -0,0 +1,77 @@
+import { instantiate } from "../wabt-wrapper.js";
+import * as assert from "../assert.js";
+
+const watGetGlobalsSameTempReturnCall = `
+(module
+  (tag $e (param f32))
+  (global $f0 (mut f32)(f32.const 2.0))
+  (global $i0 (mut i64)(i64.const 1))
+  (func (export "test") (local i64)
+    try
+      local.get 0
+      global.get $f0
+      return_call $bar
+    catch_all
+      global.get $i0
+      return_call $foo
+    end
+  )
+  (func $foo)
+  (func $bar)
+)
+`;
+
+const watGetGlobalCatchArgSameTempReturnCall = `
+(module
+  (tag $e (param i64))
+  (global $f0 (mut f32)(f32.const 2.0))
+  (global $i0 (mut i64)(i64.const 1))
+  (func (export "test") (result i64)
+    try (result i64)
+      global.get $f0
+      global.get $f0
+      return_call $bar
+    catch $e
+      global.get $i0
+      i64.add
+    end
+  )
+  (func $bar (param f32) (result i64) i64.const 3)
+)
+`;
+
+const watGetGlobalsSameTempUnreachable = `
+(module
+  (tag $e (param f32))
+  (global $f0 (mut f32)(f32.const 2.0))
+  (global $i0 (mut i64)(i64.const 1))
+  (func (export "test") (local i64)
+    try
+      local.get 0
+      global.get $f0
+      unreachable
+    catch_all
+      global.get $i0
+      return_call $foo
+    end
+  )
+  (func $foo)
+  (func $bar)
+)
+`;
+
+async function runOne(wat, isUnreachable) {
+    const instance = await instantiate(wat, {}, { exceptions: true, tail_call: true });
+    const {test} = instance.exports;
+    if (isUnreachable) {
+        assert.throws(() => {
+            test();
+        }, WebAssembly.RuntimeError, `Unreachable code`);
+    } else {
+        test();
+    }
+}
+
+assert.asyncTest(runOne(watGetGlobalsSameTempReturnCall, false));
+assert.asyncTest(runOne(watGetGlobalCatchArgSameTempReturnCall, false));
+assert.asyncTest(runOne(watGetGlobalsSameTempUnreachable, true));
\ No newline at end of file
Loading diff…

Original Bug Report

The reporter's bug is still restricted on the tracker.
On This Page