PatchGap
Firefox · SpiderMonkey

CVE-2024-8389

Memory Corruption in SpiderMonkey
High CVSS 9.8 firefox Memory Corruption 🔧 Commit mapped

Overview

High
Severity
9.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionMemory safety bugs present in Firefox 129. Some of these bugs showed evidence of memory corruption and we presume that with enough effort some of these could have been exploited to run arbitrary code.
ComponentSpiderMonkey
Bug ClassMemory Corruption
Tracker1907230
Fix commitb3e54698b3ad (firefox) +16/-3
CWECWE-787, CWE-119 (Out-of-bounds write, Buffer bounds error)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H
CISA KEVNot listed
Creditedthe Mozilla Fuzzing Team, Andrew McCreight
Disclosed2024-09-03

Background

Sandbox
an isolated JavaScript global (a JSObject created by xpc::CreateSandboxObject) used by privileged XPConnect code to run script with a controlled set of capabilities.
Sandbox metadata
arbitrary caller-supplied data attached to a sandbox global via xpc::SetSandboxMetadata and later retrieved with xpc::GetSandboxMetadata.
`nsresult`
Mozilla’s status code type; a value satisfying NS_FAILED signals that an operation did not complete successfully.
`[[nodiscard]]`
a C++ attribute that makes the compiler warn when a function’s return value is ignored, used to enforce that error codes are checked.

Root Cause Analysis

In xpc::CreateSandboxObject, the call to xpc::SetSandboxMetadata returned an nsresult that was silently discarded, so the function proceeded to JS_FireOnNewGlobalObject and returned a fully-constructed sandbox even when metadata initialization had failed. Setting metadata can fail — for example when a pending JS exception or an out-of-memory / conversion error leaves the JSContext in an error state — and continuing past that point violates the invariant that a sandbox is only handed back to callers once all of its initialization steps succeeded and the context is clean. Firing the new-global-object hook and running further script against a half-initialized global, while an exception may already be pending on cx, is what leads to the memory-unsafe state exercised by the crashtest.

The fix captures the nsresult from xpc::SetSandboxMetadata, and on NS_FAILED returns early before the global is finalized and exposed, so no code operates on an inconsistently initialized sandbox. Marking xpc::SetSandboxMetadata [[nodiscard]] guarantees the compiler will flag any future caller that repeats the mistake of ignoring the result.

Key insight
The single core mistake was ignoring the failure status returned by xpc::SetSandboxMetadata, allowing sandbox construction to continue after a failed, exception-leaving step; the fix propagates that nsresult with an early return and enforces the check permanently via [[nodiscard]].

Attack Path

  1. Trigger sandbox creation Cause privileged code to enter xpc::CreateSandboxObject, which reaches the metadata-initialization step.
  2. Force metadata failure Arrange for xpc::SetSandboxMetadata to fail (e.g. by driving the JSContext into an error/exception or OOM state during the metadata conversion), leaving a pending exception on cx.
  3. Continue past the error Because the returned nsresult was discarded, execution falls through to JSAutoRealm and JS_FireOnNewGlobalObject on the incompletely initialized global.
  4. Run script on the broken global The event and DOM manipulation in the crashtest (dispatchEvent on document.doctype, then document.write("<audio>")) drives further work against the inconsistent state, producing memory corruption.
  5. Escalate With controlled heap/state manipulation, the resulting memory corruption is potentially shaped into arbitrary code execution.

Impact Assessment

An attacker who can steer allocation and exception state gains a memory-corruption primitive in a privileged context where XPConnect sandboxes are created, which Mozilla presumes could be leveraged toward arbitrary code execution. Exploitation requires driving xpc::SetSandboxMetadata to fail at the right moment and then interacting with the partially-constructed global, as demonstrated by the 1907230.html crashtest. The impact is rated high given the privileged process context and the code-execution potential.

Files Changed

  • js/xpconnect/crashtests/1907230.html
  • js/xpconnect/crashtests/crashtests.list
  • js/xpconnect/src/Sandbox.cpp
  • js/xpconnect/src/xpcprivate.h

Audit Directions

  • Discarded `nsresult` at initialization boundaries
    Look for other object/global construction paths that call fallible helpers without checking the returned nsresult before firing hooks or returning the object.
  • Missing `[[nodiscard]]` on fallible APIs
    Audit XPConnect and SpiderMonkey helper declarations that return nsresult or bool status but lack [[nodiscard]], since these invite silent error dropping.
  • Continuation with a pending exception
    Search for code that proceeds to run script (JS_FireOnNewGlobalObject, callbacks, DOM dispatch) after a step that can leave a pending exception on the JSContext without clearing or propagating it.

Patch

view commit
diff --git a/js/xpconnect/src/Sandbox.cpp b/js/xpconnect/src/Sandbox.cpp
index 0af438d7ce4..3dc7866929e 100644
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -1500,7 +1500,10 @@ nsresult xpc::CreateSandboxObject(JSContext* cx, MutableHandleValue vp,
   // about:memory may use that information
   xpc::SetLocationForGlobal(sandbox, options.sandboxName);
 
-  xpc::SetSandboxMetadata(cx, sandbox, options.metadata);
+  nsresult rv = xpc::SetSandboxMetadata(cx, sandbox, options.metadata);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   JSAutoRealm ar(cx, sandbox);
   JS_FireOnNewGlobalObject(cx, sandbox);
diff --git a/js/xpconnect/src/xpcprivate.h b/js/xpconnect/src/xpcprivate.h
index 38d7516c361..ce7b60b3a72 100644
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -2442,8 +2442,9 @@ nsresult EvalInSandbox(JSContext* cx, JS::HandleObject sandbox,
 nsresult GetSandboxMetadata(JSContext* cx, JS::HandleObject sandboxArg,
                             JS::MutableHandleValue rval);
 
-nsresult SetSandboxMetadata(JSContext* cx, JS::HandleObject sandboxArg,
-                            JS::HandleValue metadata);
+[[nodiscard]] nsresult SetSandboxMetadata(JSContext* cx,
+                                          JS::HandleObject sandboxArg,
+                                          JS::HandleValue metadata);
 
 bool CreateObjectIn(JSContext* cx, JS::HandleValue vobj,
                     CreateObjectInOptions& options,
diff --git a/js/xpconnect/crashtests/1907230.html b/js/xpconnect/crashtests/1907230.html
new file mode 100644
index 00000000000..9a72f688826
--- /dev/null
+++ b/js/xpconnect/crashtests/1907230.html
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<script>
+window.addEventListener("load", () => {
+  let a = new UIEvent("load", {"bubbles": true})
+  try { document.doctype.dispatchEvent(a) } catch (e) {}
+  document.write("<audio>")
+})
+</script>
diff --git a/js/xpconnect/crashtests/crashtests.list b/js/xpconnect/crashtests/crashtests.list
index 129c34db277..6cf1bf4e435 100644
--- a/js/xpconnect/crashtests/crashtests.list
+++ b/js/xpconnect/crashtests/crashtests.list
@@ -53,3 +53,4 @@ pref(dom.use_xbl_scopes_for_remote_xul,true) load 898939.html
 pref(security.fileuri.strict_origin_policy,false) load 938297.html
 load 977538.html
 load 1577573.html
+load 1907230.html
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/js/xpconnect/crashtests/1907230.html b/js/xpconnect/crashtests/1907230.html
new file mode 100644
index 00000000000..9a72f688826
--- /dev/null
+++ b/js/xpconnect/crashtests/1907230.html
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<script>
+window.addEventListener("load", () => {
+  let a = new UIEvent("load", {"bubbles": true})
+  try { document.doctype.dispatchEvent(a) } catch (e) {}
+  document.write("<audio>")
+})
+</script>
diff --git a/js/xpconnect/crashtests/crashtests.list b/js/xpconnect/crashtests/crashtests.list
index 129c34db277..6cf1bf4e435 100644
--- a/js/xpconnect/crashtests/crashtests.list
+++ b/js/xpconnect/crashtests/crashtests.list
@@ -53,3 +53,4 @@ pref(dom.use_xbl_scopes_for_remote_xul,true) load 898939.html
 pref(security.fileuri.strict_origin_policy,false) load 938297.html
 load 977538.html
 load 1577573.html
+load 1907230.html
Loading diff…
On This Page