CVE-2024-9396
Overview
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_HEADERtag, 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.
SCTAG_TM_TRANSFERRING at the start of reading and refusing any buffer already in that state.Attack Path
- 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. - Trigger a partial read
A first
deserializeis made to fail mid-way throughJSStructuredCloneReader::readTransferMap(e.g. via a read-transfer error) after some resources have already had their ownership moved out of the buffer. - Re-deserialize the same buffer
Because the header was never advanced past
SCTAG_TM_UNREAD, the attacker callsdeserializeon the same buffer again, and the reader re-enters the transfer loop. - 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.
- 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
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifjs/src/vm/StructuredClone.cpp |
modified |
Files Changed
js/public/StructuredClone.hjs/public/friend/ErrorNumbers.msgjs/src/jit-test/tests/structured-clone/transferable-cleanup.jsjs/src/vm/StructuredClone.cpp
Audit Directions
- Non-atomic state transitionsLook 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 deserializationAudit 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 callbacksReview 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.
Patch
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;
Regression Test / PoC
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;