Firefox · SpiderMonkey
CVE-2024-9396
Memory Corruption in SpiderMonkey
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
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
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/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;
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));
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…
References
On This Page