From 6ce4d0eb16dff57d8736cb0fc2824832fc12c2a0 Mon Sep 17 00:00:00 2001
From: Benedikt Meurer <[email protected]>
Date: Wed, 16 Sep 2026 19:00:53 +0200
Subject: [PATCH] [inspector] Build custom preview JsonML directly as protocol::ListValue

Previously, substituteObjectTags mutated the page-owned JsonML array in
place to insert RemoteObject wrappers and relied on v8::JSON::Stringify
for serialization. This allowed page scripts to observe secret objectIds
and mutate already-validated JsonML nodes via microtasks (TOCTOU),
.toJSON() methods, or getters during traversal, as well as bypass tag
checks using boxed String objects or extra elements.

Construct a C++ protocol::ListValue tree directly during traversal
without modifying the untrusted JS array, require tag names to be
primitive strings, and serialize via CBOR-to-JSON conversion. In
bodyCallback, suppress microtasks and return a detached JS object tree
parsed from the C++ JSON string.

TAG=agy
CONV=6fae0907-92db-4e9f-8e0b-f94a35beca6b

Fixed: 553931056
Change-Id: I97153f115a317899c14a36c6b986e6e62099e406
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8412032
Reviewed-by: Simon Zünd <[email protected]>
Commit-Queue: Simon Zünd <[email protected]>
Cr-Commit-Position: refs/heads/main@{#109896}
---
diff --git a/src/inspector/custom-preview.cc b/src/inspector/custom-preview.cc
index cc4ce76..b9133b2 100644
--- a/src/inspector/custom-preview.cc
+++ b/src/inspector/custom-preview.cc
@@ -56,7 +56,128 @@
 
 bool substituteObjectTags(int sessionId, const String16& groupName,
                           v8::Local<v8::Context> context,
-                          v8::Local<v8::Array> jsonML, int maxDepth) {
+                          v8::Local<v8::Array> jsonML, int maxDepth,
+                          std::unique_ptr<protocol::ListValue>* result);
+
+bool jsonMLValueToProtocolValue(int sessionId, const String16& groupName,
+                                v8::Local<v8::Context> context,
+                                v8::Local<v8::Value> value, int maxDepth,
+                                std::unique_ptr<protocol::Value>* result) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  v8::TryCatch tryCatch(isolate);
+
+  if (value->IsProxy()) {
+    reportError(context, tryCatch, "Proxy values are not allowed in JSONML");
+    return false;
+  }
+  if (value->IsNull() || value->IsUndefined() || value->IsFunction() ||
+      value->IsSymbol()) {
+    *result = protocol::Value::null();
+    return true;
+  }
+  if (value->IsBoolean()) {
+    *result =
+        protocol::FundamentalValue::create(value.As<v8::Boolean>()->Value());
+    return true;
+  }
+  if (value->IsNumber()) {
+    double doubleValue = value.As<v8::Number>()->Value();
+    if (doubleValue == 0.0) {
+      *result = protocol::FundamentalValue::create(0);
+      return true;
+    }
+    if (!std::isfinite(doubleValue)) {
+      *result = protocol::Value::null();
+      return true;
+    }
+    if (doubleValue >= std::numeric_limits<int>::min() &&
+        doubleValue <= std::numeric_limits<int>::max()) {
+      int intValue = static_cast<int>(doubleValue);
+      if (intValue == doubleValue) {
+        *result = protocol::FundamentalValue::create(intValue);
+        return true;
+      }
+    }
+    *result = protocol::FundamentalValue::create(doubleValue);
+    return true;
+  }
+  if (value->IsString()) {
+    *result = protocol::StringValue::create(
+        toProtocolString(isolate, value.As<v8::String>()));
+    return true;
+  }
+  if (value->IsArray()) {
+    std::unique_ptr<protocol::ListValue> listResult;
+    if (!substituteObjectTags(sessionId, groupName, context,
+                              value.As<v8::Array>(), maxDepth, &listResult)) {
+      return false;
+    }
+    *result = std::move(listResult);
+    return true;
+  }
+  if (value->IsObject()) {
+    if (maxDepth <= 0) {
+      reportError(context, tryCatch,
+                  "Too deep hierarchy of inlined custom previews");
+      return false;
+    }
+    v8::Local<v8::Object> object = value.As<v8::Object>();
+    auto dict = protocol::DictionaryValue::create();
+    v8::Local<v8::Array> propertyNames;
+    if (!object->GetOwnPropertyNames(context).ToLocal(&propertyNames)) {
+      reportError(context, tryCatch);
+      return false;
+    }
+    uint32_t length = propertyNames->Length();
+    for (uint32_t i = 0; i < length; ++i) {
+      v8::Local<v8::Value> name;
+      if (!propertyNames->Get(context, i).ToLocal(&name)) {
+        reportError(context, tryCatch);
+        return false;
+      }
+      if (name->IsString()) {
+        v8::Maybe<bool> hasRealNamedProperty =
+            object->HasRealNamedProperty(context, name.As<v8::String>());
+        if (hasRealNamedProperty.IsNothing()) {
+          reportError(context, tryCatch);
+          return false;
+        }
+        if (!hasRealNamedProperty.FromJust()) continue;
+      }
+      v8::Local<v8::String> propertyName;
+      if (!name->ToString(context).ToLocal(&propertyName)) {
+        reportError(context, tryCatch);
+        return false;
+      }
+      v8::Local<v8::Value> property;
+      if (!object->Get(context, name).ToLocal(&property)) {
+        reportError(context, tryCatch);
+        return false;
+      }
+      if (property->IsUndefined() || property->IsFunction() ||
+          property->IsSymbol()) {
+        continue;
+      }
+      std::unique_ptr<protocol::Value> protocolProperty;
+      if (!jsonMLValueToProtocolValue(sessionId, groupName, context, property,
+                                      maxDepth - 1, &protocolProperty)) {
+        return false;
+      }
+      dict->setValue(toProtocolString(isolate, propertyName),
+                     std::move(protocolProperty));
+    }
+    *result = std::move(dict);
+    return true;
+  }
+  reportError(context, tryCatch, "Cannot serialize value in JSONML");
+  return false;
+}
+
+bool substituteObjectTags(int sessionId, const String16& groupName,
+                          v8::Local<v8::Context> context,
+                          v8::Local<v8::Array> jsonML, int maxDepth,
+                          std::unique_ptr<protocol::ListValue>* result) {
+  *result = protocol::ListValue::create();
   if (!jsonML->Length()) return true;
   v8::Isolate* isolate = v8::Isolate::GetCurrent();
   v8::TryCatch tryCatch(isolate);
@@ -72,9 +193,20 @@
     reportError(context, tryCatch);
     return false;
   }
+  if (firstValue->IsProxy()) {
+    reportError(context, tryCatch, "Proxy values are not allowed in JSONML");
+    return false;
+  }
+  if (!firstValue->IsString()) {
+    reportError(context, tryCatch, "tag name must be a string");
+    return false;
+  }
   v8::Local<v8::String> objectLiteral = toV8String(isolate, "object");
-  if (jsonML->Length() == 2 && firstValue->IsString() &&
-      firstValue.As<v8::String>()->StringEquals(objectLiteral)) {
+  if (firstValue.As<v8::String>()->StringEquals(objectLiteral)) {
+    if (jsonML->Length() != 2) {
+      reportError(context, tryCatch, "object tag should have 2 elements");
+      return false;
+    }
     v8::Local<v8::Value> attributesValue;
     if (!jsonML->Get(context, 1).ToLocal(&attributesValue)) {
       reportError(context, tryCatch);
@@ -129,22 +261,19 @@
       reportError(context, tryCatch, "cannot wrap value");
       return false;
     }
-    std::vector<uint8_t> json;
-    v8_crdtp::json::ConvertCBORToJSON(v8_crdtp::SpanFrom(wrapper->Serialize()),
-                                      &json);
-    v8::Local<v8::Value> jsonWrapper;
-    v8_inspector::StringView serialized(json.data(), json.size());
-    if (!v8::JSON::Parse(context, toV8String(isolate, serialized))
-             .ToLocal(&jsonWrapper)) {
+    std::vector<uint8_t> cbor = wrapper->Serialize();
+    std::unique_ptr<protocol::Value> protocolWrapper =
+        protocol::Value::parseBinary(cbor.data(), cbor.size());
+    if (!protocolWrapper) {
       reportError(context, tryCatch, "cannot wrap value");
       return false;
     }
-    if (jsonML->Set(context, 1, jsonWrapper).IsNothing()) {
-      reportError(context, tryCatch);
-      return false;
-    }
+    (*result)->pushValue(protocol::StringValue::create("object"));
+    (*result)->pushValue(std::move(protocolWrapper));
   } else {
-    for (uint32_t i = 0; i < jsonML->Length(); ++i) {
+    (*result)->pushValue(protocol::StringValue::create(
+        toProtocolString(isolate, firstValue.As<v8::String>())));
+    for (uint32_t i = 1; i < jsonML->Length(); ++i) {
       v8::Local<v8::Value> value;
       if (!jsonML->Get(context, i).ToLocal(&value)) {
         reportError(context, tryCatch);
@@ -155,10 +284,25 @@
                     "Proxy values are not allowed in JSONML");
         return false;
       }
-      if (value->IsArray() && value.As<v8::Array>()->Length() > 0 &&
-          !substituteObjectTags(sessionId, groupName, context,
-                                value.As<v8::Array>(), maxDepth - 1)) {
-        return false;
+      if (value->IsArray()) {
+        std::unique_ptr<protocol::ListValue> childList;
+        if (value.As<v8::Array>()->Length() > 0) {
+          if (!substituteObjectTags(sessionId, groupName, context,
+                                    value.As<v8::Array>(), maxDepth - 1,
+                                    &childList)) {
+            return false;
+          }
+        } else {
+          childList = protocol::ListValue::create();
+        }
+        (*result)->pushValue(std::move(childList));
+      } else {
+        std::unique_ptr<protocol::Value> protocolElement;
+        if (!jsonMLValueToProtocolValue(sessionId, groupName, context, value,
+                                        maxDepth - 1, &protocolElement)) {
+          return false;
+        }
+        (*result)->pushValue(std::move(protocolElement));
       }
     }
   }
@@ -169,6 +313,8 @@
   v8::Isolate* isolate = v8::Isolate::GetCurrent();
   v8::TryCatch tryCatch(isolate);
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
+  v8::MicrotasksScope microtasksScope(context,
+                                      v8::MicrotasksScope::kDoNotRunMicrotasks);
   v8::Local<v8::Object> bodyConfig =
       info.DataV2().As<v8::Value>().As<v8::Object>();
 
@@ -253,14 +399,24 @@
     return;
   }
   v8::Local<v8::Array> jsonML = formattedValue.As<v8::Array>();
-  if (jsonML->Length() &&
-      !substituteObjectTags(
+  std::unique_ptr<protocol::ListValue> protocolJsonML;
+  if (!substituteObjectTags(
           sessionIdValue.As<v8::Int32>()->Value(),
           toProtocolString(isolate, groupNameValue.As<v8::String>()), context,
-          jsonML, kMaxCustomPreviewDepth)) {
+          jsonML, kMaxCustomPreviewDepth, &protocolJsonML)) {
     return;
   }
-  info.GetReturnValue().Set(jsonML);
+  std::vector<uint8_t> json;
+  v8_crdtp::json::ConvertCBORToJSON(
+      v8_crdtp::SpanFrom(protocolJsonML->Serialize()), &json);
+  v8::Local<v8::Value> detachedJsonML;
+  v8_inspector::StringView serialized(json.data(), json.size());
+  if (!v8::JSON::Parse(context, toV8String(isolate, serialized))
+           .ToLocal(&detachedJsonML)) {
+    reportError(context, tryCatch, "cannot serialize body");
+    return;
+  }
+  info.GetReturnValue().Set(detachedJsonML);
 }
 }  // anonymous namespace
 
@@ -342,16 +498,17 @@
     }
     bool hasBody = hasBodyValue->ToBoolean(isolate)->Value();
 
-    if (jsonML->Length() && !substituteObjectTags(sessionId, groupName, context,
-                                                  jsonML, maxDepth)) {
+    std::unique_ptr<protocol::ListValue> protocolJsonML;
+    if (!substituteObjectTags(sessionId, groupName, context, jsonML, maxDepth,
+                              &protocolJsonML)) {
       return;
     }
 
-    v8::Local<v8::String> header;
-    if (!v8::JSON::Stringify(context, jsonML).ToLocal(&header)) {
-      reportError(context, tryCatch);
-      return;
-    }
+    std::vector<uint8_t> json;
+    v8_crdtp::json::ConvertCBORToJSON(
+        v8_crdtp::SpanFrom(protocolJsonML->Serialize()), &json);
+    v8_inspector::StringView serialized(json.data(), json.size());
+    String16 header = toString16(serialized);
 
     v8::Local<v8::Function> bodyFunction;
     if (hasBody) {
@@ -397,9 +554,7 @@
         return;
       }
     }
-    *preview = CustomPreview::create()
-                   .setHeader(toProtocolString(isolate, header))
-                   .build();
+    *preview = CustomPreview::create().setHeader(header).build();
     if (!bodyFunction.IsEmpty()) {
       V8InspectorImpl* inspector =
           static_cast<V8InspectorImpl*>(v8::debug::GetInspector(isolate));
diff --git a/test/inspector/runtime/regress-553931056-expected.txt b/test/inspector/runtime/regress-553931056-expected.txt
new file mode 100644
index 0000000..c2f59a9
--- /dev/null
+++ b/test/inspector/runtime/regress-553931056-expected.txt
@@ -0,0 +1,92 @@
+Tests that custom preview formatters prevent TOCTOU and sanitization bypasses
+1. Test microtask TOCTOU attack on body() and verify no objectId leak to page JS..
+customPreview: ["span",{},"Header ok"]
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : object
+            value : [
+                [0] : div
+                [1] : {
+                }
+                [2] : [
+                    [0] : object
+                    [1] : {
+                        className : Object
+                        customPreview : {
+                            bodyGetterId : <bodyGetterId>
+                            header : ["span",{},"Header ok"]
+                        }
+                        description : Object
+                        objectId : <objectId>
+                        type : object
+                    }
+                ]
+                [3] : [
+                    [0] : span
+                    [1] : {
+                    }
+                    [2] : benign
+                ]
+            ]
+        }
+    }
+}
+microtaskObservedObjectId in page JS: undefined
+2. Test toJSON() bypass attempt in header() and body()..
+customPreview: ["span",{},["span",{},"initial"]]
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : object
+            value : [
+                [0] : div
+                [1] : {
+                }
+                [2] : [
+                    [0] : span
+                    [1] : {
+                    }
+                    [2] : initial
+                ]
+            ]
+        }
+    }
+}
+toJSONCalled: false
+3. Test getter mutating earlier validated element during traversal..
+customPreview: ["div",{},["span",{},"safe-child"],"trigger"]
+4. Test new String("object") unboxing bypass attempt..
+{
+    method : Runtime.consoleAPICalled
+    params : {
+        args : [
+            [0] : {
+                type : string
+                value : Custom Formatter Failed: Uncaught tag name must be a string
+            }
+        ]
+        executionContextId : <executionContextId>
+        timestamp : <timestamp>
+        type : error
... (file diff truncated)

diff --git a/test/inspector/runtime/regress-553931056-expected.txt b/test/inspector/runtime/regress-553931056-expected.txt
new file mode 100644
index 0000000..c2f59a9
--- /dev/null
+++ b/test/inspector/runtime/regress-553931056-expected.txt
@@ -0,0 +1,92 @@
+Tests that custom preview formatters prevent TOCTOU and sanitization bypasses
+1. Test microtask TOCTOU attack on body() and verify no objectId leak to page JS..
+customPreview: ["span",{},"Header ok"]
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : object
+            value : [
+                [0] : div
+                [1] : {
+                }
+                [2] : [
+                    [0] : object
+                    [1] : {
+                        className : Object
+                        customPreview : {
+                            bodyGetterId : <bodyGetterId>
+                            header : ["span",{},"Header ok"]
+                        }
+                        description : Object
+                        objectId : <objectId>
+                        type : object
+                    }
+                ]
+                [3] : [
+                    [0] : span
+                    [1] : {
+                    }
+                    [2] : benign
+                ]
+            ]
+        }
+    }
+}
+microtaskObservedObjectId in page JS: undefined
+2. Test toJSON() bypass attempt in header() and body()..
+customPreview: ["span",{},["span",{},"initial"]]
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : object
+            value : [
+                [0] : div
+                [1] : {
+                }
+                [2] : [
+                    [0] : span
+                    [1] : {
+                    }
+                    [2] : initial
+                ]
+            ]
+        }
+    }
+}
+toJSONCalled: false
+3. Test getter mutating earlier validated element during traversal..
+customPreview: ["div",{},["span",{},"safe-child"],"trigger"]
+4. Test new String("object") unboxing bypass attempt..
+{
+    method : Runtime.consoleAPICalled
+    params : {
+        args : [
+            [0] : {
+                type : string
+                value : Custom Formatter Failed: Uncaught tag name must be a string
+            }
+        ]
+        executionContextId : <executionContextId>
+        timestamp : <timestamp>
+        type : error
+    }
+}
+No customPreview generated
+5. Test ["object", forged, "extra"] (length != 2) bypass attempt..
+{
+    method : Runtime.consoleAPICalled
+    params : {
+        args : [
+            [0] : {
+                type : string
+                value : Custom Formatter Failed: Uncaught object tag should have 2 elements
+            }
+        ]
+        executionContextId : <executionContextId>
+        timestamp : <timestamp>
+        type : error
+    }
+}
+No customPreview generated
diff --git a/test/inspector/runtime/regress-553931056.js b/test/inspector/runtime/regress-553931056.js
new file mode 100644
index 0000000..78d75d2
--- /dev/null
+++ b/test/inspector/runtime/regress-553931056.js
@@ -0,0 +1,143 @@
+// Copyright 2026 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Regression test for b/553931056.
+
+const { session, contextGroup, Protocol } =
+  InspectorTest.start('Tests that custom preview formatters prevent TOCTOU and sanitization bypasses');
+
+(async function test() {
+  Protocol.Runtime.enable();
+  Protocol.Runtime.setCustomObjectFormatterEnabled({ enabled: true });
+  Protocol.Runtime.onConsoleAPICalled(m => InspectorTest.logMessage(m));
+
+  InspectorTest.log('1. Test microtask TOCTOU attack on body() and verify no objectId leak to page JS..');
+  await Protocol.Runtime.evaluate({
+    expression: `
+      var microtaskObservedObjectId = 'not-run';
+      this.devtoolsFormatters = [{
+        header: () => ['span', {}, 'Header ok'],
+        hasBody: () => true,
+        body: () => {
+          const trustedSlot = ['object', { object: { real: 1 } }];
+          const victimSlot = ['span', {}, 'benign'];
+          const tree = ['div', {}, trustedSlot, victimSlot];
+          Promise.resolve().then(() => {
+            microtaskObservedObjectId = trustedSlot[1].objectId;
+            victimSlot.length = 0;
+            victimSlot.push('object', {
+              type: 'object',
+              objectId: 'forged-id',
+              description: 'forged-remote-object'
+            });
+          });
+          return tree;
+        }
+      }];
+    `
+  });
+  let evalRes = await Protocol.Runtime.evaluate({ expression: '({})', generatePreview: true });
+  await dumpCustomPreviewWithBody(evalRes);
+  let leakRes = await Protocol.Runtime.evaluate({ expression: 'String(microtaskObservedObjectId)' });
+  InspectorTest.log('microtaskObservedObjectId in page JS: ' + leakRes.result.result.value);
+
+  InspectorTest.log('2. Test toJSON() bypass attempt in header() and body()..');
+  await Protocol.Runtime.evaluate({
+    expression: `
+      var toJSONCalled = false;
+      const sneakyChild = ['span', {}, 'initial'];
+      sneakyChild.toJSON = () => {
+        toJSONCalled = true;
+        return ['object', { type: 'object', objectId: 'forged-via-toJSON' }];
+      };
+      this.devtoolsFormatters = [{
+        header: () => ['span', {}, sneakyChild],
+        hasBody: () => true,
+        body: () => ['div', {}, sneakyChild]
+      }];
+    `
+  });
+  evalRes = await Protocol.Runtime.evaluate({ expression: '({})', generatePreview: true });
+  await dumpCustomPreviewWithBody(evalRes);
+  let toJSONRes = await Protocol.Runtime.evaluate({ expression: 'toJSONCalled' });
+  InspectorTest.log('toJSONCalled: ' + toJSONRes.result.result.value);
+
+  InspectorTest.log('3. Test getter mutating earlier validated element during traversal..');
+  await Protocol.Runtime.evaluate({
+    expression: `
+      this.devtoolsFormatters = [{
+        header: () => {
+          const firstChild = ['span', {}, 'safe-child'];
+          const tree = ['div', {}, firstChild];
+          Object.defineProperty(tree, '3', {
+            enumerable: true,
+            get() {
+              firstChild[0] = 'object';
+              firstChild[1] = { type: 'object', objectId: 'forged-via-getter' };
+              firstChild.length = 2;
+              return 'trigger';
+            }
+          });
+          return tree;
+        },
+        hasBody: () => false
+      }];
+    `
+  });
+  evalRes = await Protocol.Runtime.evaluate({ expression: '({})', generatePreview: true });
+  dumpCustomPreview(evalRes);
+
+  InspectorTest.log('4. Test new String("object") unboxing bypass attempt..');
+  await Protocol.Runtime.evaluate({
+    expression: `
+      this.devtoolsFormatters = [{
+        header: () => ['span', {}, [new String('object'), { type: 'object', objectId: 'forged-string-obj' }]],
+        hasBody: () => false
+      }];
+    `
+  });
+  evalRes = await Protocol.Runtime.evaluate({ expression: '({})', generatePreview: true });
+  dumpCustomPreview(evalRes);
+
+  InspectorTest.log('5. Test ["object", forged, "extra"] (length != 2) bypass attempt..');
+  await Protocol.Runtime.evaluate({
+    expression: `
+      this.devtoolsFormatters = [{
+        header: () => ['span', {}, ['object', { type: 'object', objectId: 'forged-len-3' }, 'extra']],
+        hasBody: () => false
+      }];
+    `
+  });
+  evalRes = await Protocol.Runtime.evaluate({ expression: '({})', generatePreview: true });
+  dumpCustomPreview(evalRes);
+
+  InspectorTest.completeTest();
+})();
+
+function dumpCustomPreview(result) {
+  const remoteObject = result.result.result;
+  if (remoteObject.customPreview) {
+    InspectorTest.log('customPreview: ' + remoteObject.customPreview.header);
+  } else {
+    InspectorTest.log('No customPreview generated');
+  }
+}
+
+async function dumpCustomPreviewWithBody(result) {
+  const remoteObject = result.result.result;
+  if (remoteObject.customPreview) {
+    InspectorTest.log('customPreview: ' + remoteObject.customPreview.header);
+    if (remoteObject.customPreview.bodyGetterId) {
+      const body = await Protocol.Runtime.callFunctionOn({
+        objectId: remoteObject.objectId,
+        functionDeclaration: 'function(bodyGetter) { return bodyGetter.call(this); }',
+        arguments: [{ objectId: remoteObject.customPreview.bodyGetterId }],
+        returnByValue: true
+      });
+      InspectorTest.logMessage(body);
+    }
+  } else {
+    InspectorTest.log('No customPreview generated');
+  }
+}