CachedString::m_jsString is not protected from GC in CloneDeserializer.

https://bugs.webkit.org/show_bug.cgi?id=267971 rdar://120531481 Reviewed by Chris Dumez. The fix is simply to protect it with the m_keepAliveBuffer. Also moved the m_keepAliveBuffer from CloneSerializer to CloneBase. Previously, I thought that only the serializer needs it. Now, we have a case where the deserializer does too. * LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map-expected.txt: Added. * LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map.html: Added. * Source/WebCore/bindings/js/SerializedScriptValue.cpp: (WebCore::CloneDeserializer::CachedString::jsString): (WebCore::CloneDeserializer::readTerminal): Originally-landed-as: 272448.347@safari-7618-branch (5546b2e). rdar://124555235 Canonical link: https://commits.webkit.org/276167@main
WebKit · Mar 15, 2024 · 442482a · 442482a
1 parent 1e46cd5
commit 442482a
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 6 deletions.
diff --git a/LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map-expected.txt b/LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map-expected.txt
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: PASS
+
diff --git a/LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map.html b/LayoutTests/js/structuredClone/structured-clone-of-CachedString-in-map.html
@@ -0,0 +1,38 @@
+<body>
+<script>
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function main() {
+    const string = 'Expected result';
+
+    const map = new Map();
+    map.set('free', string);
+
+    map.set('tmp', {
+        get a() {
+            map.delete('free');
+            map.set('free', 0x1234);
+
+            for (let i = 0; i < 0x10; i++)
+                map.set('gc1_' + i, new ArrayBuffer(1024 * 1024 * 0x10));
+
+            for (let i = 0; i < 0x2000; i++)
+                map.set('gc2_' + i, new Date());
+
+            map.set('expected', string);
+        }
+    });
+
+    const result = structuredClone(map);
+
+    if (result.get('expected') != "Expected result")
+        console.log("FAIL");
+    else
+        console.log("PASS");
+}
+
+main();
+</script>
+</body>
diff --git a/Source/WebCore/bindings/js/SerializedScriptValue.cpp b/Source/WebCore/bindings/js/SerializedScriptValue.cpp
@@ -937,6 +937,7 @@ class CloneBase {
 
     JSGlobalObject* const m_lexicalGlobalObject;
     bool m_failed;
+    MarkedArgumentBuffer m_keepAliveBuffer;
     MarkedArgumentBuffer m_objectPool;
 #if ASSERT_ENABLED
     Vector<SerializationTag> m_objectPoolTags;
@@ -2684,7 +2685,6 @@ class CloneSerializer : public CloneBase {
 #endif
     SerializationForStorage m_forStorage;
 
-    MarkedArgumentBuffer m_keepAliveBuffer;
 #if ASSERT_ENABLED
     bool m_didSeeComplexCases { false };
 #endif
@@ -3080,10 +3080,13 @@ class CloneDeserializer : public CloneBase {
         {
         }
 
-        JSValue jsString(JSGlobalObject* lexicalGlobalObject)
+        JSValue jsString(CloneDeserializer& deserializer)
         {
-            if (!m_jsString)
-                m_jsString = JSC::jsString(lexicalGlobalObject->vm(), m_string);
+            if (!m_jsString) {
+                auto& vm = deserializer.m_lexicalGlobalObject->vm();
+                m_jsString = JSC::jsString(vm, m_string);
+                deserializer.m_keepAliveBuffer.appendWithCrashOnOverflow(m_jsString);
+            }
             return m_jsString;
         }
         const String& string() { return m_string; }
@@ -4949,15 +4952,15 @@ class CloneDeserializer : public CloneBase {
             CachedStringRef cachedString;
             if (!readStringData(cachedString))
                 return JSValue();
-            return cachedString->jsString(m_lexicalGlobalObject);
+            return cachedString->jsString(*this);
         }
         case EmptyStringTag:
             return jsEmptyString(m_lexicalGlobalObject->vm());
         case StringObjectTag: {
             CachedStringRef cachedString;
             if (!readStringData(cachedString))
                 return JSValue();
-            StringObject* obj = constructString(m_lexicalGlobalObject->vm(), m_globalObject, cachedString->jsString(m_lexicalGlobalObject));
+            StringObject* obj = constructString(m_lexicalGlobalObject->vm(), m_globalObject, cachedString->jsString(*this));
             addToObjectPool<StringObjectTag>(obj);
             return obj;
         }