Medium CVSS 8.8 firefox Memory Corruption 🔧 Commit mapped

Overview

Medium
Severity
8.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionIt is currently unknown if this issue is exploitable but a condition may arise where the structured clone of certain objects could lead to memory corruption.
ComponentSpiderMonkey
Bug ClassMemory Corruption
Tracker1912471
Fix commit55434158f89e (firefox) +44/-10
CWECWE-119 (Buffer bounds error)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H
CISA KEVNot listed
CreditedNils Bars
Disclosed2024-10-01

Background

Structured clone
SpiderMonkey’s serialization mechanism (JSStructuredCloneReader/JSStructuredCloneWriter) that deep-copies JavaScript values across contexts, boundaries, or storage.
Transferable
an object (such as an ArrayBuffer) whose underlying resource is moved rather than copied during a structured clone, leaving the source neutered and transferring ownership to the destination.
Transfer map
the region of the serialized byte stream, headed by an SCTAG_TRANSFER_MAP_HEADER tag, that records each transferable’s ownership pointer and extra data so the reader can hand the resource over exactly once.
`TransferableMapHeader`
the enum stored alongside the header tag that tracks whether the transfer map has been read yet (SCTAG_TM_UNREAD) or already consumed (SCTAG_TM_TRANSFERRED).

Root Cause Analysis

When JSStructuredCloneReader::readTransferMap processed a serialized buffer, it only distinguished the two states SCTAG_TM_UNREAD and SCTAG_TM_TRANSFERRED, and it did not rewrite the header to a “read in progress” state until after all transferables had been handed over. Because the state transition was not atomic with respect to reading, a buffer whose transfer map was partially read — for example when an error interrupted the loop after ownership of some resources had already been moved out — could be deserialized a second time while its header still read SCTAG_TM_UNREAD. On that second pass the reader would attempt to transfer resources it had already given away, producing duplicate ownership and use-after-free / double-free style memory corruption.

The fix introduces a third state, SCTAG_TM_TRANSFERRING, written into the header the moment reading begins; a subsequent read that observes SCTAG_TM_TRANSFERRING now reports JS_SCERR_TRANSFERABLE_TWICE and bails out before invoking any transfer callbacks, so no resource can be transferred twice.

Key insight
The core mistake was a missing intermediate state in the transfer-map header: reading was not marked in-progress, so a partially or fully consumed buffer could be re-read and re-transfer resources it no longer owned. The fix closes this by writing SCTAG_TM_TRANSFERRING at the start of reading and refusing any buffer already in that state.

Attack Path

  1. Craft a transferable clone An attacker obtains or constructs a structured-clone buffer containing a transfer map for one or more transferables such as ArrayBuffers.
  2. Trigger a partial read A first deserialize is made to fail mid-way through JSStructuredCloneReader::readTransferMap (e.g. via a read-transfer error) after some resources have already had their ownership moved out of the buffer.
  3. Re-deserialize the same buffer Because the header was never advanced past SCTAG_TM_UNREAD, the attacker calls deserialize on the same buffer again, and the reader re-enters the transfer loop.
  4. Double-transfer the resource The reader hands over pointers to resources that were already transferred (or already freed) on the first pass, creating duplicated ownership and dangling references.
  5. Exploit the corruption Subsequent use of the doubly-owned or freed memory yields a use-after-free / double-free primitive that can be groomed toward memory corruption.

Impact Assessment

An attacker who can drive structured-clone deserialization of an attacker-influenced buffer gains a double-free / use-after-free condition on transferable resources within the content process running SpiderMonkey. The precondition is the ability to re-deserialize a buffer whose transfer map was left in a partially-read state, typically by forcing an error during a prior read. Mozilla rated the issue moderate, and while confirmed exploitability was unknown, the corruption could plausibly be leveraged for further compromise.

Changed Functions

FunctionChangeNotes
if
js/src/vm/StructuredClone.cpp
modified

Files Changed

  • js/public/StructuredClone.h
  • js/public/friend/ErrorNumbers.msg
  • js/src/jit-test/tests/structured-clone/transferable-cleanup.js
  • js/src/vm/StructuredClone.cpp

Audit Directions

  • Non-atomic state transitions
    Look for serialized-stream or resource-ownership state machines whose “consumed” flag is written only after processing completes, leaving a window for re-entry on the unadvanced state.
  • Re-entrant deserialization
    Audit any code path that permits deserializing or replaying the same buffer more than once, especially where an early error can abort processing after side effects (ownership moves, frees) have occurred.
  • Ownership-transfer callbacks
    Review transferable/ownership hand-off callbacks to confirm each resource can be moved at most once and that error paths reconcile which side owns the resource before any retry.
diff --git a/js/public/StructuredClone.h b/js/public/StructuredClone.h
index ca13bcf2811..c60f8ac734c 100644
--- a/js/public/StructuredClone.h
+++ b/js/public/StructuredClone.h
@@ -757,6 +757,7 @@ class JS_PUBLIC_API JSAutoStructuredCloneBuffer {
 #define JS_SCERR_WASM_NO_TRANSFER 6
 #define JS_SCERR_NOT_CLONABLE 7
 #define JS_SCERR_NOT_CLONABLE_WITH_COOP_COEP 8
+#define JS_SCERR_TRANSFERABLE_TWICE 9
 
 JS_PUBLIC_API bool JS_ReadUint32Pair(JSStructuredCloneReader* r, uint32_t* p1,
                                      uint32_t* p2);
diff --git a/js/public/friend/ErrorNumbers.msg b/js/public/friend/ErrorNumbers.msg
index 34bb7f5164a..0719b4f8afc 100644
--- a/js/public/friend/ErrorNumbers.msg
+++ b/js/public/friend/ErrorNumbers.msg
@@ -558,6 +558,7 @@ MSG_DEF(JSMSG_SC_BAD_CLONE_VERSION,    0, JSEXN_ERR, "unsupported structured clo
 MSG_DEF(JSMSG_SC_BAD_SERIALIZED_DATA,  1, JSEXN_INTERNALERR, "bad serialized structured data ({0})")
 MSG_DEF(JSMSG_SC_DUP_TRANSFERABLE,     0, JSEXN_TYPEERR, "duplicate transferable for structured clone")
 MSG_DEF(JSMSG_SC_NOT_TRANSFERABLE,     0, JSEXN_TYPEERR, "invalid transferable array for structured clone")
+MSG_DEF(JSMSG_SC_TRANSFERABLE_TWICE,   0, JSEXN_TYPEERR, "structured clone cannot transfer twice")
 MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE,     0, JSEXN_TYPEERR, "unsupported type for structured data")
 MSG_DEF(JSMSG_SC_NOT_CLONABLE,         1, JSEXN_TYPEERR, "The {0} object cannot be serialized. The Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy HTTP headers will enable this in the future.")
 MSG_DEF(JSMSG_SC_NOT_CLONABLE_WITH_COOP_COEP, 1, JSEXN_TYPEERR, "The {0} object cannot be serialized. The Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy HTTP headers can be used to enable this.")
diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp
index f19bd743f21..1197b6a106d 100644
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@@ -178,17 +178,25 @@ enum StructuredDataType : uint32_t {
 
 /*
  * Format of transfer map:
- *   <SCTAG_TRANSFER_MAP_HEADER, TransferableMapHeader(UNREAD|TRANSFERRED)>
- *   numTransferables (64 bits)
- *   array of:
- *     <SCTAG_TRANSFER_MAP_*, TransferableOwnership>
- *     pointer (64 bits)
- *     extraData (64 bits), eg byte length for ArrayBuffers
+ *   - <SCTAG_TRANSFER_MAP_HEADER, UNREAD|TRANSFERRING|TRANSFERRED>
+ *   - numTransferables (64 bits)
+ *   - array of:
+ *     - <SCTAG_TRANSFER_MAP_*, TransferableOwnership> pointer (64
+ *       bits)
+ *     - extraData (64 bits), eg byte length for ArrayBuffers
+ *     - any data written for custom transferables
  */
 
 // Data associated with an SCTAG_TRANSFER_MAP_HEADER that tells whether the
-// contents have been read out yet or not.
-enum TransferableMapHeader { SCTAG_TM_UNREAD = 0, SCTAG_TM_TRANSFERRED };
+// contents have been read out yet or not. TRANSFERRING is for the case where we
+// have started but not completed reading, which due to errors could mean that
+// there are things still owned by the clone buffer that need to be released, so
+// discarding should not just be skipped.
+enum TransferableMapHeader {
+  SCTAG_TM_UNREAD = 0,
+  SCTAG_TM_TRANSFERRING,
+  SCTAG_TM_TRANSFERRED
+};
 
 static inline uint64_t PairToUInt64(uint32_t tag, uint32_t data) {
   return uint64_t(data) | (uint64_t(tag) << 32);
@@ -705,6 +713,10 @@ static void ReportDataCloneError(JSContext* cx,
       errorNumber = JSMSG_SC_SHMEM_TRANSFERABLE;
       break;
 
+    case JS_SCERR_TRANSFERABLE_TWICE:
+      errorNumber = JSMSG_SC_TRANSFERABLE_TWICE;
+      break;
+
     case JS_SCERR_TYPED_ARRAY_DETACHED:
       errorNumber = JSMSG_TYPED_ARRAY_DETACHED;
       break;
@@ -3408,11 +3420,21 @@ bool JSStructuredCloneReader::readTransferMap() {
     return in.reportTruncated();
   }
 
+  auto transferState = static_cast<TransferableMapHeader>(data);
+
   if (tag != SCTAG_TRANSFER_MAP_HEADER ||
-      TransferableMapHeader(data) == SCTAG_TM_TRANSFERRED) {
+      transferState == SCTAG_TM_TRANSFERRED) {
     return true;
   }
 
+  if (transferState == SCTAG_TM_TRANSFERRING) {
+    ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE_TWICE, closure);
+    return false;
+  }
+
+  headerPos.write(
+      PairToUInt64(SCTAG_TRANSFER_MAP_HEADER, SCTAG_TM_TRANSFERRING));
+
   uint64_t numTransferables;
   MOZ_ALWAYS_TRUE(in.readPair(&tag, &data));
   if (!in.read(&numTransferables)) {
@@ -3532,7 +3554,7 @@ bool JSStructuredCloneReader::readTransferMap() {
 #ifdef DEBUG
   SCInput::getPair(headerPos.peek(), &tag, &data);
   MOZ_ASSERT(tag == SCTAG_TRANSFER_MAP_HEADER);
-  MOZ_ASSERT(TransferableMapHeader(data) != SCTAG_TM_TRANSFERRED);
+  MOZ_ASSERT(TransferableMapHeader(data) == SCTAG_TM_TRANSFERRING);
 #endif
   headerPos.write(
       PairToUInt64(SCTAG_TRANSFER_MAP_HEADER, SCTAG_TM_TRANSFERRED));
diff --git a/js/src/jit-test/tests/structured-clone/transferable-cleanup.js b/js/src/jit-test/tests/structured-clone/transferable-cleanup.js
index e55cb480f43..d701a6a8a68 100644
--- a/js/src/jit-test/tests/structured-clone/transferable-cleanup.js
+++ b/js/src/jit-test/tests/structured-clone/transferable-cleanup.js
@@ -160,6 +160,15 @@ function testMultiWithDeserializeReadTransferErrorHelper(g, BASE, desc) {
     } catch (e) {
         assertEq(e.message.includes("invalid transferable"), true);
     }
+
+    try {
+        // This fails without logging anything, since the re-transfer will be caught
+        // by looking at its header before calling any callbacks.
+        let clone = deserialize(s);
+    } catch (e) {
+        assertEq(e.message.includes("cannot transfer twice"), true);
+    }
+
     s = null;
     gc();
     printTrace(arguments.callee.name, g, BASE, obj.log, "deserialize");
@@ -170,6 +179,7 @@ function testMultiWithDeserializeReadTransferErrorHelper(g, BASE, desc) {
         // which comes before the main reading. obj transfer data is now owned by its
         // clone. obj3 transfer data was not successfully handed over to a new object,
         // so it is still owned by the clone buffer and must be discarded with freeTransfer.
+        // 'F' means the data is freed.
         BASE + 3, "F",
     ], "deserialize " + desc);
     obj.log = null;
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/js/src/jit-test/tests/structured-clone/transferable-cleanup.js b/js/src/jit-test/tests/structured-clone/transferable-cleanup.js
index e55cb480f43..d701a6a8a68 100644
--- a/js/src/jit-test/tests/structured-clone/transferable-cleanup.js
+++ b/js/src/jit-test/tests/structured-clone/transferable-cleanup.js
@@ -160,6 +160,15 @@ function testMultiWithDeserializeReadTransferErrorHelper(g, BASE, desc) {
     } catch (e) {
         assertEq(e.message.includes("invalid transferable"), true);
     }
+
+    try {
+        // This fails without logging anything, since the re-transfer will be caught
+        // by looking at its header before calling any callbacks.
+        let clone = deserialize(s);
+    } catch (e) {
+        assertEq(e.message.includes("cannot transfer twice"), true);
+    }
+
     s = null;
     gc();
     printTrace(arguments.callee.name, g, BASE, obj.log, "deserialize");
@@ -170,6 +179,7 @@ function testMultiWithDeserializeReadTransferErrorHelper(g, BASE, desc) {
         // which comes before the main reading. obj transfer data is now owned by its
         // clone. obj3 transfer data was not successfully handed over to a new object,
         // so it is still owned by the clone buffer and must be discarded with freeTransfer.
+        // 'F' means the data is freed.
         BASE + 3, "F",
     ], "deserialize " + desc);
     obj.log = null;
Loading diff…