Chrome · DevTools
CVE-2026-79266
UAF in DevTools
Overview
Medium
Severity
8.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- `V8InspectorSession`
- the V8 inspector object a DevTools session uses to wrap, unwrap, and inspect JS objects; it is owned via a
std::shared_ptrcreated byinspector->connectShared. - Inspector agents
- per-domain objects (e.g.
InspectorDOMAgent,InspectorDOMDebuggerAgent,InspectorAnimationAgent) that hold a reference to the V8 session to build protocol object representations. - `V8SessionHolder`
- the newly introduced holder type (
v8_session_holder.h) that owns a reference to the V8 inspector session and is passed to agents so they co-own it for the duration of use. - Custom `devtoolsFormatter`
- page-supplied JS invoked while DevTools builds object descriptions; it runs re-entrantly during protocol handling and can trigger further protocol activity such as a session disconnect.
Root Cause Analysis
Inspector agents held the V8 inspector session as a non-owning raw pointer (v8_inspector::V8InspectorSession* v8_session_), while DevToolsSession owned it through a std::shared_ptr. When a protocol method such as DOMDebugger.getEventListeners built object representations, it could invoke a page-defined custom devtoolsFormatter, re-entering script execution; that script (or a paused-debugger callback) could disconnect the session, dropping the sole owning shared_ptr and destroying the V8InspectorSession while an agent was still mid-operation. The agent then continued to dereference its dangling raw pointer — a use-after-free — violating the invariant that the session outlives any agent operation using it.
The fix replaces the raw pointer with a V8SessionHolder that is passed by value into each agent through InspectorBaseAgent::Init and stored, so an agent co-owns the session while building objects; DevToolsSession::V8Session() now returns a holder, DevToolsSession::ConnectToV8 wraps connectShared in a V8SessionHolder, and Dispose calls v8_session_.reset(). Because the agent keeps the session alive across the re-entrant formatter call, a mid-operation disconnect no longer frees the object out from under it.
Key insight
The core mistake was letting agents hold a non-owning raw pointer to a
shared_ptr-owned V8 session that could be freed by a re-entrant devtoolsFormatter-triggered disconnect; the fix gives agents shared ownership via V8SessionHolder so the session is retained while objects are built.Attack Path
- Open two CDP sessions
An attacker with DevTools/CDP access to a target attaches two protocol sessions to the same page (as the regression test does with
sessionandsession2). - Install a re-entrant formatter
The page registers a custom
window.devtoolsFormatterswhoseheader()executes attacker-controlled JS (the PoC runsdebugger;), and enables custom object formatters. - Invoke object building
The attacker calls
DOMDebugger.getEventListenerson an object, causing the DOM/DOMDebugger agent to build wrapped objects and thereby invoke the custom formatter re-entrantly. - Disconnect mid-build
From the second session (or the paused-debugger callback), the attacker disconnects, dropping the owning
shared_ptrand freeing theV8InspectorSessionwhile the agent still holds its raw pointer. - Trigger the dangling dereference
The agent resumes and dereferences the freed session (e.g. via
wrapObject/unwrapObject), producing a use-after-free (memory corruption or crash) (exploitation beyond the crash is(inferred)).
Impact Assessment
An actor able to drive the DevTools/CDP protocol against a target can free the
V8InspectorSession during re-entrant custom-formatter execution and cause a use-after-free in the renderer process, giving at least a controlled crash and potential memory corruption. Preconditions are DevTools/CDP access (e.g. an inspectable target or automation) plus a page that installs a custom devtoolsFormatter, as encoded in the shipped regression test. The advisory classifies this as a use-after-free in DevTools and does not report in-the-wild exploitation; escalation to code execution is (inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
playback_rate_third_party/blink/renderer/core/inspector/inspector_animation_agent.cc |
modified | |
CORE_EXPORTthird_party/blink/renderer/core/inspector/inspector_animation_agent.h |
modified | |
ifthird_party/blink/renderer/core/inspector/inspector_dom_agent.cc |
modified |
Files Changed
third_party/blink/renderer/core/inspector/inspector_network_agent.ccthird_party/blink/renderer/core/inspector/inspector_overlay_agent.hthird_party/blink/renderer/core/inspector/inspector_log_agent.ccthird_party/blink/renderer/core/inspector/inspector_emulation_agent_test.ccthird_party/blink/renderer/core/inspector/inspector_web_mcp_agent.ccthird_party/blink/renderer/core/inspector/inspector_network_agent.hPRESUBMIT.pythird_party/blink/renderer/core/inspector/devtools_session.ccthird_party/blink/renderer/modules/modules_initializer.ccthird_party/blink/renderer/core/inspector/v8_session_holder.hthird_party/blink/renderer/core/inspector/inspector_base_agent.hthird_party/blink/renderer/core/inspector/inspector_dom_agent.ccthird_party/blink/renderer/core/inspector/inspector_overlay_agent.ccthird_party/blink/renderer/core/inspector/inspector_page_agent.hthird_party/blink/renderer/modules/indexeddb/inspector_indexed_db_agent.ccthird_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.ccthird_party/blink/renderer/core/inspector/inspector_animation_agent.ccthird_party/blink/renderer/core/inspector/build.gnithird_party/blink/renderer/core/inspector/inspector_event_breakpoints_agent.ccthird_party/blink/web_tests/inspector-protocol/debugger/domdebugger-getEventListeners-custom-formatter-disconnect.jsthird_party/blink/renderer/core/inspector/inspector_css_agent_test.ccthird_party/blink/renderer/core/inspector/inspector_io_agent.hthird_party/blink/renderer/core/inspector/inspector_web_mcp_agent.hthird_party/blink/renderer/core/inspector/inspector_page_agent.ccthird_party/blink/renderer/core/exported/web_dev_tools_agent_impl.ccthird_party/blink/renderer/core/inspector/inspector_dom_agent.hthird_party/blink/renderer/core/inspector/inspector_animation_agent.hthird_party/blink/renderer/core/inspector/inspector_io_agent.ccthird_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.hthird_party/blink/renderer/core/inspector/devtools_session.hthird_party/blink/renderer/core/inspector/worker_inspector_controller.ccthird_party/blink/renderer/core/inspector/inspector_event_breakpoints_agent.hthird_party/blink/renderer/modules/indexeddb/inspector_indexed_db_agent.hthird_party/blink/web_tests/inspector-protocol/debugger/domdebugger-getEventListeners-custom-formatter-disconnect-expected.txtthird_party/blink/renderer/core/inspector/inspector_log_agent.h
Audit Directions
- Raw pointers to shared-owned objectsAudit for non-owning raw pointers (or references) cached from a
shared_ptr/unique_ptrthat can be dropped by re-entrant callbacks; convert them to owning holders where the callback can outlive or free the owner. - Re-entrancy via page-controlled callbacksReview any inspector/protocol path that invokes page-supplied JS (custom formatters, event handlers,
debuggerpauses) mid-operation and confirm objects used before and after the callback are retained across it. - Disconnect/detach during operationTrace session/agent lifetimes to ensure a disconnect or
Detach/Disposereached during an in-progress protocol method cannot free state the method continues to use.
Patch
From 01c9ad7fe24638f468ec728736fd4cd74abcd9b4 Mon Sep 17 00:00:00 2001 From: Andrey Kosyakov <[email protected]> Date: Thu, 23 Jul 2026 13:14:35 -0700 Subject: [PATCH] Retain V8 session while building objects for event listeners See also: https://chromium-review.git.corp.google.com/c/v8/v8/+/8135751 (either CL fixes the problem, but we want both for potential other problems) Bug: 537145191 Change-Id: I3b5e56a94f8a32ee57025b7d832c633ec46ff797 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8126504 Reviewed-by: Alex Rudenko <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Auto-Submit: Andrey Kosyakov <[email protected]> Commit-Queue: Andrey Kosyakov <[email protected]> Cr-Commit-Position: refs/heads/main@{#1667363} --- diff --git a/PRESUBMIT.py b/PRESUBMIT.py index da9a1f26..0f42933 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -791,7 +791,7 @@ # Needed for interop with third-party library. r'^third_party/blink/renderer/core/typed_arrays/array_buffer/' + r'array_buffer_contents\.(cc|h)', - r'^third_party/blink/renderer/core/inspector/devtools_session\.h', + r'^third_party/blink/renderer/core/inspector/v8_session_holder\.h', r'^third_party/blink/renderer/core/typed_arrays/dom_array_buffer\.cc', '^third_party/blink/renderer/bindings/core/v8/' + 'v8_wasm_response_extensions.cc', diff --git a/third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.cc b/third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.cc index e65bcb0..450ed92 100644 --- a/third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.cc +++ b/third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.cc @@ -332,14 +332,14 @@ session->ConnectToV8(main_thread_debugger->GetV8Inspector(), context_group_id); - InspectorDOMAgent* dom_agent = session->CreateAndAppend<InspectorDOMAgent>( - isolate, inspected_frames, session->V8Session()); + InspectorDOMAgent* dom_agent = + session->CreateAndAppend<InspectorDOMAgent>(isolate, inspected_frames); session->CreateAndAppend<InspectorLayerTreeAgent>(inspected_frames, this); InspectorNetworkAgent* network_agent = - session->CreateAndAppend<InspectorNetworkAgent>(inspected_frames, nullptr, - session->V8Session()); + session->CreateAndAppend<InspectorNetworkAgent>(inspected_frames, + nullptr); session->CreateAndAppend<InspectorCrashReportContextAgent>(inspected_frames); auto* css_agent = session->CreateAndAppend<InspectorCSSAgent>( @@ -347,41 +347,36 @@ resource_content_loader_.Get(), resource_container_.Get()); InspectorDOMDebuggerAgent* dom_debugger_agent = - session->CreateAndAppend<InspectorDOMDebuggerAgent>(isolate, dom_agent, - session->V8Session()); + session->CreateAndAppend<InspectorDOMDebuggerAgent>(isolate, dom_agent); - session->CreateAndAppend<InspectorEventBreakpointsAgent>( - session->V8Session()); + session->CreateAndAppend<InspectorEventBreakpointsAgent>(); session->CreateAndAppend<InspectorPerformanceAgent>(inspected_frames); session->CreateAndAppend<InspectorDOMSnapshotAgent>(inspected_frames, dom_debugger_agent); - session->CreateAndAppend<InspectorAnimationAgent>(inspected_frames, css_agent, - session->V8Session()); + session->CreateAndAppend<InspectorAnimationAgent>(inspected_frames, + css_agent); session->CreateAndAppend<InspectorMemoryAgent>(inspected_frames); if (base::FeatureList::IsEnabled(features::kDevToolsWebMCPSupport)) { - session->CreateAndAppend<InspectorWebMCPAgent>(inspected_frames, - session->V8Session()); + session->CreateAndAppend<InspectorWebMCPAgent>(inspected_frames); } auto* page_agent = session->CreateAndAppend<InspectorPageAgent>( inspected_frames, this, resource_content_loader_.Get(), - session->V8Session(), session->script_to_evaluate_on_load(), - session->InjectedScriptManager()); + session->script_to_evaluate_on_load(), session->InjectedScriptManager()); session->CreateAndAppend<InspectorLogAgent>( &inspected_frames->Root()->GetPage()->GetConsoleMessageStorage(), - inspected_frames->Root()->GetPerformanceMonitor(), session->V8Session()); + inspected_frames->Root()->GetPerformanceMonitor()); InspectorOverlayAgent* overlay_agent = session->CreateAndAppend<InspectorOverlayAgent>( - web_local_frame_impl_.Get(), inspected_frames, session->V8Session(), - dom_agent); + web_local_frame_impl_.Get(), inspected_frames, dom_agent); - session->CreateAndAppend<InspectorIOAgent>(isolate, session->V8Session()); + session->CreateAndAppend<InspectorIOAgent>(isolate); session->CreateAndAppend<InspectorAuditsAgent>( network_agent, diff --git a/third_party/blink/renderer/core/inspector/devtools_session.cc b/third_party/blink/renderer/core/inspector/devtools_session.cc index fc7e306..8d47059 100644 --- a/third_party/blink/renderer/core/inspector/devtools_session.cc +++ b/third_party/blink/renderer/core/inspector/devtools_session.cc @@ -244,7 +244,7 @@ const auto& cbor = v8_session_state_cbor_.Get(); const auto* reattach_state = session_state_.ReattachState(); - v8_session_ = inspector->connectShared( + v8_session_ = V8SessionHolder(inspector->connectShared( context_group_id, this, v8_inspector::StringView(cbor.data(), cbor.size()), client_is_trusted_ ? v8_inspector::V8Inspector::kFullyTrusted @@ -255,7 +255,7 @@ ConvertEmbedderState( reattach_state ? reattach_state->browser_originating_session_state.get() - : nullptr)); + : nullptr))); injected_script_manager_->SetV8Session(v8_session_.get()); } @@ -266,7 +266,7 @@ void DevToolsSession::Append(InspectorAgent* agent) { agents_.push_back(agent); agent->Init(agent_->probe_sink_.Get(), inspector_backend_dispatcher_.get(), - &session_state_); + &session_state_, v8_session_); } void DevToolsSession::Detach() { diff --git a/third_party/blink/renderer/core/inspector/devtools_session.h b/third_party/blink/renderer/core/inspector/devtools_session.h index fcf0fa1b..28d1007 100644 --- a/third_party/blink/renderer/core/inspector/devtools_session.h +++ b/third_party/blink/renderer/core/inspector/devtools_session.h @@ -7,6 +7,7 @@ #include <memory> #include <type_traits> + #include "base/functional/callback.h" #include "base/task/sequenced_task_runner.h" #include "mojo/public/cpp/bindings/pending_associated_receiver.h" @@ -16,6 +17,7 @@ #include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/inspector/inspector_session_state.h" #include "third_party/blink/renderer/core/inspector/protocol/forward.h" +#include "third_party/blink/renderer/core/inspector/v8_session_holder.h" #include "third_party/blink/renderer/platform/heap/collection_support/heap_vector.h" #include "third_party/blink/renderer/platform/heap/garbage_collected.h" #include "third_party/blink/renderer/platform/mojo/heap_mojo_associated_receiver.h" @@ -75,7 +77,7 @@ ~DevToolsSession() override; void ConnectToV8(v8_inspector::V8Inspector*, int context_group_id); - v8_inspector::V8InspectorSession* V8Session() { return v8_session_.get(); } + V8SessionHolder V8Session() { return v8_session_; } template <typename Agent, typename... Args> Agent* CreateAndAppend(Args&&... args) { @@ -176,7 +178,7 @@ HeapMojoAssociatedRemote<mojom::blink::DevToolsSessionHost> host_remote_{ nullptr}; IOSession* io_session_; - std::shared_ptr<v8_inspector::V8InspectorSession> v8_session_; + V8SessionHolder v8_session_; std::unique_ptr<protocol::UberDispatcher> inspector_backend_dispatcher_; InspectorSessionState session_state_; HeapVector<Member<InspectorAgent>> agents_; diff --git a/third_party/blink/renderer/core/inspector/inspector_animation_agent.cc b/third_party/blink/renderer/core/inspector/inspector_animation_agent.cc index 3266b43..5485949 100644 --- a/third_party/blink/renderer/core/inspector/inspector_animation_agent.cc +++ b/third_party/blink/renderer/core/inspector/inspector_animation_agent.cc @@ -86,11 +86,9 @@ InspectorAnimationAgent::InspectorAnimationAgent( InspectedFrames* inspected_frames, - InspectorCSSAgent* css_agent, - v8_inspector::V8InspectorSession* v8_session) + InspectorCSSAgent* css_agent) : inspected_frames_(inspected_frames), css_agent_(css_agent), - v8_session_(v8_session), is_cloning_(false), enabled_(&agent_state_, /*default_value=*/false), playback_rate_(&agent_state_, /*default_value=*/1.0) { @@ -509,9 +507,9 @@ ScriptState::Scope scope(script_state); static const char kAnimationObjectGroup[] = "animation"; - v8_session_->releaseObjectGroup( + V8Session()->releaseObjectGroup( ToV8InspectorStringView(kAnimationObjectGroup)); - *result = v8_session_->wrapObject( + *result = V8Session()->wrapObject( script_state->GetContext(), ToV8Traits<Animation>::ToV8(script_state, animation), ToV8InspectorStringView(kAnimationObjectGroup), diff --git a/third_party/blink/renderer/core/inspector/inspector_animation_agent.h b/third_party/blink/renderer/core/inspector/inspector_animation_agent.h index 257766bf..53ad866 100644 --- a/third_party/blink/renderer/core/inspector/inspector_animation_agent.h +++ b/third_party/blink/renderer/core/inspector/inspector_animation_agent.h @@ -28,9 +28,7 @@ class CORE_EXPORT InspectorAnimationAgent final : public InspectorBaseAgent<protocol::Animation::Metainfo> { public: - InspectorAnimationAgent(InspectedFrames*, - InspectorCSSAgent*, - v8_inspector::V8InspectorSession*); + InspectorAnimationAgent(InspectedFrames*, InspectorCSSAgent*); InspectorAnimationAgent(const InspectorAnimationAgent&) = delete; InspectorAnimationAgent& operator=(const InspectorAnimationAgent&) = delete; @@ -114,7 +112,6 @@ Member<InspectedFrames> inspected_frames_; Member<InspectorCSSAgent> css_agent_; - v8_inspector::V8InspectorSession* v8_session_; // Keeps track of the snapshot of animations that are sent to the frontend. // The snapshots are used to check whether to send an `animationUpdated` event // when a blink::Animation instance is updated. diff --git a/third_party/blink/renderer/core/inspector/inspector_base_agent.h b/third_party/blink/renderer/core/inspector/inspector_base_agent.h index 967915f..519c1a79 100644 --- a/third_party/blink/renderer/core/inspector/inspector_base_agent.h +++ b/third_party/blink/renderer/core/inspector/inspector_base_agent.h @@ -35,6 +35,7 @@ #include "third_party/blink/renderer/core/core_probe_sink.h" #include "third_party/blink/renderer/core/inspector/inspector_session_state.h" #include "third_party/blink/renderer/core/inspector/protocol/protocol.h" +#include "third_party/blink/renderer/core/inspector/v8_session_holder.h" #include "third_party/blink/renderer/platform/heap/garbage_collected.h" #include "third_party/blink/renderer/platform/wtf/forward.h" #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h" @@ -55,7 +56,8 @@ virtual void Init(CoreProbeSink*, protocol::UberDispatcher*, - InspectorSessionState*) = 0; + InspectorSessionState*, + V8SessionHolder) = 0; virtual void Dispose() = 0; }; @@ -65,13 +67,15 @@ public: void Init(CoreProbeSink* instrumenting_agents, protocol::UberDispatcher* dispatcher, - InspectorSessionState* session_state) override { + InspectorSessionState* session_state, + V8SessionHolder v8_session) override { instrumenting_agents_ = instrumenting_agents; frontend_.reset( new typename DomainMetainfo::FrontendClass(dispatcher->channel())); DomainMetainfo::DispatcherClass::wire(dispatcher, this); agent_state_.InitFrom(session_state); + v8_session_ = std::move(v8_session); } protocol::Response disable() override { @@ -82,6 +86,7 @@ disable(); frontend_.reset(); instrumenting_agents_ = nullptr; + v8_session_.reset(); } void Trace(Visitor* visitor) const override { @@ -89,6 +94,8 @@ InspectorAgent::Trace(visitor); } + V8SessionHolder V8Session() const { return v8_session_; } + protected: InspectorBaseAgent() : agent_state_(DomainMetainfo::domainName) {} ~InspectorBaseAgent() override { @@ -97,11 +104,13 @@ typename DomainMetainfo::FrontendClass* GetFrontend() const { return frontend_.get(); } + Member<CoreProbeSink> instrumenting_agents_; InspectorAgentState agent_state_; private: std::unique_ptr<typename DomainMetainfo::FrontendClass> frontend_; + V8SessionHolder v8_session_; }; } // namespace blink diff --git a/third_party/blink/renderer/core/inspector/inspector_dom_agent.cc b/third_party/blink/renderer/core/inspector/inspector_dom_agent.cc index 5192c4c..032ca701 100644 --- a/third_party/blink/renderer/core/inspector/inspector_dom_agent.cc +++ b/third_party/blink/renderer/core/inspector/inspector_dom_agent.cc @@ -466,13 +466,10 @@ NOTREACHED(); } -InspectorDOMAgent::InspectorDOMAgent( - v8::Isolate* isolate, - InspectedFrames* inspected_frames, - v8_inspector::V8InspectorSession* v8_session) +InspectorDOMAgent::InspectorDOMAgent(v8::Isolate* isolate, + InspectedFrames* inspected_frames) : isolate_(isolate), inspected_frames_(inspected_frames), - v8_session_(v8_session), document_node_to_id_map_(MakeGarbageCollected<NodeToIdMap>()), last_node_id_(1), suppress_attribute_modified_event_(false), @@ -1650,10 +1647,10 @@ v8::Local<v8::Value> value; v8::Local<v8::Context> context; std::unique_ptr<v8_inspector::StringBuffer> error; - if (!v8_session_) { + if (!V8Session()) { return protocol::Response::ServerError("The agent has been detached"); } - if (!v8_session_->unwrapObject(&error, ToV8InspectorStringView(object_id), + if (!V8Session()->unwrapObject(&error, ToV8InspectorStringView(object_id), &value, &context, nullptr)) { return protocol::Response::ServerError( ToCoreString(std::move(error)).Utf8()); @@ -1930,8 +1927,8 @@ if (!node) return protocol::Response::ServerError("No node with given id found"); // This should only be called via CDP, so agent should not be detached. - CHECK(v8_session_); - *result = ResolveNode(v8_session_, node, object_group_name, + CHECK(V8Session()); + *result = ResolveNode(V8Session().get(), node, object_group_name, std::move(execution_context_id)); if (!*result) { return protocol::Response::ServerError( @@ -3208,8 +3205,8 @@ if (!response.IsSuccess()) return response; // Method should only be called from CDP, so won't happen after detach. - CHECK(v8_session_); - v8_session_->addInspectedObject(std::make_unique<InspectableNode>(node)); + CHECK(V8Session()); + V8Session()->addInspectedObject(std::make_unique<InspectableNode>(node)); return protocol::Response::Success(); } @@ -3344,12 +3341,12 @@ String* path) { // Method is only called from CDP, so will not be called after Detach(). CHECK(isolate_); - CHECK(v8_session_); + CHECK(V8Session()); v8::HandleScope handles(isolate_); v8::Local<v8::Value> value; v8::Local<v8::Context> context; std::unique_ptr<v8_inspector::StringBuffer> error; - if (!v8_session_->unwrapObject(&error, ToV8InspectorStringView(object_id), + if (!V8Session()->unwrapObject(&error, ToV8InspectorStringView(object_id), &value, &context, nullptr)) { return protocol::Response::ServerError( ToCoreString(std::move(error)).Utf8()); @@ -3445,7 +3442,6 @@ void InspectorDOMAgent::Dispose() { InspectorBaseAgent<protocol::DOM::Metainfo>::Dispose(); isolate_ = nullptr; - v8_session_ = nullptr; } } // namespace blink diff --git a/third_party/blink/renderer/core/inspector/inspector_dom_agent.h b/third_party/blink/renderer/core/inspector/inspector_dom_agent.h index af543c9..1d185c9 100644 --- a/third_party/blink/renderer/core/inspector/inspector_dom_agent.h +++ b/third_party/blink/renderer/core/inspector/inspector_dom_agent.h @@ -88,9 +88,7 @@ Document*); static ShadowRoot* UserAgentShadowRoot(Node*); - InspectorDOMAgent(v8::Isolate*, - InspectedFrames*, - v8_inspector::V8InspectorSession*); + InspectorDOMAgent(v8::Isolate*, InspectedFrames*); InspectorDOMAgent(const InspectorDOMAgent&) = delete; InspectorDOMAgent& operator=(const InspectorDOMAgent&) = delete; ~InspectorDOMAgent() override; @@ -425,7 +423,6 @@ v8::Isolate* isolate_; // null after Dispose(). Member<InspectedFrames> inspected_frames_; - v8_inspector::V8InspectorSession* v8_session_; // null after Dispose(). HeapHashSet<Member<DOMListener>> dom_listeners_; Member<NodeToIdMap> document_node_to_id_map_; // Owns node mappings for dangling nodes. diff --git a/third_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.cc b/third_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.cc index 640a514..b23a7ff 100644 --- a/third_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.cc +++ b/third_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.cc ... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/third_party/blink/renderer/core/inspector/inspector_css_agent_test.cc b/third_party/blink/renderer/core/inspector/inspector_css_agent_test.cc
index b9985b08..98b839d 100644
--- a/third_party/blink/renderer/core/inspector/inspector_css_agent_test.cc
+++ b/third_party/blink/renderer/core/inspector/inspector_css_agent_test.cc
@@ -67,11 +67,10 @@
MakeGarbageCollected<InspectedFrames>(frame);
InspectorCSSAgent* agent = MakeGarbageCollected<InspectorCSSAgent>(
MakeGarbageCollected<InspectorDOMAgent>(
- GetDocument().GetExecutionContext()->GetIsolate(), inspected_frames,
- nullptr),
+ GetDocument().GetExecutionContext()->GetIsolate(),
+ inspected_frames),
inspected_frames,
- MakeGarbageCollected<InspectorNetworkAgent>(inspected_frames, nullptr,
- nullptr),
+ MakeGarbageCollected<InspectorNetworkAgent>(inspected_frames, nullptr),
MakeGarbageCollected<InspectorResourceContentLoader>(
GetDocument().GetFrame()),
MakeGarbageCollected<InspectorResourceContainer>(inspected_frames));
diff --git a/third_party/blink/renderer/core/inspector/inspector_emulation_agent_test.cc b/third_party/blink/renderer/core/inspector/inspector_emulation_agent_test.cc
index 8d190fe..929c4d3 100644
--- a/third_party/blink/renderer/core/inspector/inspector_emulation_agent_test.cc
+++ b/third_party/blink/renderer/core/inspector/inspector_emulation_agent_test.cc
@@ -99,7 +99,8 @@
auto* agent_a = MakeGarbageCollected<InspectorEmulationAgent>(
web_frame, *virtual_time_controller);
- agent_a->Init(frame->GetProbeSink(), &dispatcher_a, &session_state_a);
+ agent_a->Init(frame->GetProbeSink(), &dispatcher_a, &session_state_a,
+ V8SessionHolder());
DummyFrontendChannel channel_b;
protocol::UberDispatcher dispatcher_b(&channel_b);
@@ -108,7 +109,8 @@
auto* agent_b = MakeGarbageCollected<InspectorEmulationAgent>(
web_frame, *virtual_time_controller);
- agent_b->Init(frame->GetProbeSink(), &dispatcher_b, &session_state_b);
+ agent_b->Init(frame->GetProbeSink(), &dispatcher_b, &session_state_b,
+ V8SessionHolder());
// Initially, throttling thread should not exist (no throttling active).
EXPECT_DOUBLE_EQ(blink::scheduler::ThreadCPUThrottler::GetInstance()
@@ -209,7 +211,8 @@
auto* agent = MakeGarbageCollected<InspectorEmulationAgent>(
web_frame, *virtual_time_controller);
- agent->Init(frame->GetProbeSink(), &dispatcher, &session_state);
+ agent->Init(frame->GetProbeSink(), &dispatcher, &session_state,
+ V8SessionHolder());
double base_ms = 0;
agent->setVirtualTimePolicy(protocol::Emulation::VirtualTimePolicyEnum::Pause,
diff --git a/third_party/blink/web_tests/inspector-protocol/debugger/domdebugger-getEventListeners-custom-formatter-disconnect-expected.txt b/third_party/blink/web_tests/inspector-protocol/debugger/domdebugger-getEventListeners-custom-formatter-disconnect-expected.txt
new file mode 100644
index 0000000..36c659e
--- /dev/null
+++ b/third_party/blink/web_tests/inspector-protocol/debugger/domdebugger-getEventListeners-custom-formatter-disconnect-expected.txt
@@ -0,0 +1,2 @@
+Tests disconnect inside custom devtoolsFormatter called from DOMDebugger.getEventListeners.
+
diff --git a/third_party/blink/web_tests/inspector-protocol/debugger/domdebugger-getEventListeners-custom-formatter-disconnect.js b/third_party/blink/web_tests/inspector-protocol/debugger/domdebugger-getEventListeners-custom-formatter-disconnect.js
new file mode 100644
index 0000000..8684ef2
--- /dev/null
+++ b/third_party/blink/web_tests/inspector-protocol/debugger/domdebugger-getEventListeners-custom-formatter-disconnect.js
@@ -0,0 +1,50 @@
+(async function(/** @type {import('test_runner').TestRunner} */ testRunner) {
+ const {page, session, dp} = await testRunner.startHTML(
+ `
+ <div id='target'></div>
+ `,
+ `Tests disconnect inside custom devtoolsFormatter called from DOMDebugger.getEventListeners.`);
+
+ const session2 = await page.createSession();
+ const dp2 = session2.protocol;
+
+ await dp.Debugger.enable();
+ await dp2.Runtime.enable();
+
+ await session.evaluate(`
+ window.devtoolsFormatters = [{
+ header() {
+ debugger;
+ return null;
+ }
+ }];
+ const target = document.getElementById('target');
+ target.addEventListener('click', () => {}, false);
+ `);
+
+ const {result} = await dp2.Runtime.evaluate({
+ expression: `document.getElementById('target')`,
+ objectGroup: 'my-group'
+ });
+ const objectId = result.result.objectId;
+
+ await dp2.Runtime.setCustomObjectFormatterEnabled({enabled: true});
+
+ const completionPromise = Promise.race([
+ testRunner.browserP().Target.onceDetachedFromTarget().then(
+ () => new Promise(resolve => setTimeout(resolve, 500))),
+ dp2.DOMDebugger.getEventListeners({objectId})
+ ]);
+
+ dp.Debugger.oncePaused().then(async () => {
+ await session2.disconnect();
+ dp.Debugger.disable();
+ });
+
+ // If this resolves without hitting a breakpoint, that's fine.
+ // However, if it hits the breakpoint, it should not crash.
+ await completionPromise;
+ // Do a round-trip to renderer to assure it hasn't crashed.
+ await session.evaluate('');
+ testRunner.completeTest();
+})
Loading diff…
Original Bug Report
reported by [email protected]
Renderer Use-After-Free of V8InspectorSession in InspectorDOMDebuggerAgent
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: A potential Use-After-Free (UAF) vulnerability exists in the renderer process within InspectorDOMDebuggerAgent due to the use of a bare pointer to a V8InspectorSession that can be destroyed during a nested message loop. If the debugger session is detached while inside this nested loop, the session is freed, potentially leading to a virtual call on the freed memory upon resuming. This occurs because Blink-domain Chrome DevTools Protocol (CDP) dispatches lack a session-pinning scope.
Affected files:
third_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.ccthird_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.hthird_party/blink/renderer/core/inspector/devtools_session.ccthird_party/blink/renderer/core/inspector/devtools_session.h
Estimated timestamp from git blame: 2016-04-27
Detailed Description
A potential Use-After-Free (UAF) vulnerability has been identified in InspectorDOMDebuggerAgent. The agent holds a bare (non-raw_ptr) pointer to a v8_inspector::V8InspectorSession object. During the dispatch of certain Blink-domain Chrome DevTools Protocol (CDP) commands, virtual methods are called on this session inside loops that can re-enter page JavaScript via custom object formatters. If the DevTools session is detached while JavaScript execution is paused in a nested message loop, the underlying V8InspectorSession is deleted, leaving the agent with a dangling pointer when execution resumes.
Root Cause
-
Lack of Pinning Scope on Blink Dispatch Path: When a CDP command is executed,
DevToolsSession::DispatchProtocolCommandImplchecks whether the command can be handled by V8. If so, it uses V8’s dispatch path which implicitly instantiates aKeepSessionAliveScope(keeping the session’s reference count > 0). If it is a Blink-domain command (such asDOMDebugger.getEventListeners), it is dispatched directly viainspector_backend_dispatcher_->Dispatch(dispatchable)without any pinning scope on the stack. -
Bare Pointer Usage:
InspectorDOMDebuggerAgentstoresv8_session_as a bare raw pointer:// third_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.h v8_inspector::V8InspectorSession* v8_session_;Unlike other agents (e.g.,
InspectorDOMAgent),InspectorDOMDebuggerAgentdoes not overrideDispose()to nullifyv8_session_upon detachment. -
Nested Pause and Teardown: In
InspectorDOMDebuggerAgent::BuildObjectForEventListener,v8_session_->wrapObjectis called on the event listener’s handler function. If custom formatters are enabled, this triggers V8’sgenerateCustomPreviewwhich runs user-supplied formatter JavaScript. If adebugger;statement is encountered during formatting, the debugger pauses and enters a nested message loop (base::RunLoop(kNestableTasksAllowed).Run()).While paused in this nested loop, a detach request (such as closing DevTools or calling
chrome.debugger.detach()) will triggerDevToolsSession::Detach(). This resets the strongstd::shared_ptr<v8_inspector::V8InspectorSession>reference to0, freeing the object. When the pause loop terminates and the stack unwinds,InspectorDOMDebuggerAgentattempts to execute the next statement:value->setOriginalHandler(v8_session_->wrapObject(context, info.handler, object_group_id, false));This results in a virtual method call on the freed
v8_session_pointer.
Suggested Potential Steps to Reproduce
Note: These steps are suggested/potential based on static code analysis; our automated tooling does not have the capability to execute code.
- Load a target page containing multiple event listeners registered on
document.body. - Configure a custom object formatter on the page’s global scope:
window.devtoolsFormatters = [{ header() { debugger; return null; } }]; - Attach a
chrome.debuggerextension to the page and enable custom formatters viaRuntime.setCustomObjectFormatterEnabled(true). - Request the event listeners for
document.bodyusing theDOMDebugger.getEventListenerscommand, passing a non-emptyobjectGroupID. - Once V8 hits the programmatic breakpoint inside the custom formatter, it enters a nested message loop on pause.
- Have the extension call
chrome.debugger.detach()to disconnect the session. - Resume/quit the pause loop. The agent resumes execution in
BuildObjectForEventListenerand attempts to perform a virtual call tov8_session_->wrapObjectusing the freed pointer.
Suggested Fix
To prevent this issue, InspectorDOMDebuggerAgent should align with the design pattern of other agents (such as InspectorDOMAgent and InspectorPageAgent) by implementing a Dispose() override to clear the v8_session_ pointer upon session teardown, and adding null-checks before referencing it:
- In
InspectorDOMDebuggerAgent:void InspectorDOMDebuggerAgent::Dispose() override { v8_session_ = nullptr; InspectorBaseAgent::Dispose(); } - Add checks to verify that
v8_session_is non-null before invoking any methods on it withingetEventListenersandBuildObjectForEventListener.
Evaluated with Chrome root at commit: bf775e5d75cb9e1767e2cd02cc93efa0077d14a5
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.
References
On This Page