Bug 1912471 - Disallow deserializing structured clone buffers with transferables more than once r=iain, a=dsmith

Differential Revision: https://phabricator.services.mozilla.com/D220644

js/public/StructuredClone.h

@@ -745,6 +745,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);

js/public/friend/ErrorNumbers.msg

@@ -555,6 +555,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.")

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;

js/src/vm/StructuredClone.cpp

@@ -178,17 +178,25 @@ enum StructuredDataType : uint32_t {
 
 /*
  * Format of transfer map:
- *   <SCTAG_TRANSFER_MAP_HEADER, TransferableMapHeader(UNREAD|TRANSFERRED)>
+ *   - <SCTAG_TRANSFER_MAP_HEADER, UNREAD|TRANSFERRING|TRANSFERRED>
- *   numTransferables (64 bits)
+ *   - numTransferables (64 bits)
- *   array of:
+ *   - array of:
- *     <SCTAG_TRANSFER_MAP_*, TransferableOwnership>
+ *     - <SCTAG_TRANSFER_MAP_*, TransferableOwnership> pointer (64
- *     pointer (64 bits)
+ *       bits)
- *     extraData (64 bits), eg byte length for ArrayBuffers
+ *     - 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.
+// contents have been read out yet or not. TRANSFERRING is for the case where we
-enum TransferableMapHeader { SCTAG_TM_UNREAD = 0, SCTAG_TM_TRANSFERRED };
+// 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);
@@ -701,6 +709,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;
@@ -3350,11 +3362,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)) {
@@ -3475,7 +3497,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));