Critical chrome UAF 🔧 Commit mapped

Overview

Critical
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactUse after free in Network
DescriptionUse after free in Network
ComponentNetwork
Sub-componentNet
Bug ClassUAF
Tracker568826881
Fix commit7dddd41a3f94 (chromium/src) +177/-3
CISA KEVNot listed
CreditedGoogle
Disclosed2026-10-13

Background

`Group` / `group_map_`
per-GroupId state holding idle sockets and in-flight ConnectJobs. RemoveGroup erases a Group from group_map_; iterators and raw Group*s into this map must stay valid for the duration of an operation.
Tunneled / proxied `StreamSocket` (e.g. `SpdyProxyClientSocket`)
a socket that rides on a shared SpdySession. Destroying it can synchronously drain that SpdySession.
`OnConnectJobComplete`
pool callback that fires when a ConnectJob finishes (including failing); it can modify or remove Group entries in group_map_.
Reentrancy hazard
destroying a proxied socket mid-operation can synchronously fail pending ConnectJobs on the shared session, reentrantly calling back into the pool.

Root Cause Analysis

The invariant is that closing/destroying a socket inside a pool operation must not synchronously mutate group_map_ (or free the Group the caller is still iterating). The old code destroyed sockets inline: idle-socket cleanup erased the socket with it = idle_sockets->erase(it) after the unique_ptr dropped, ReleaseSocket/the release path did socket.reset(), and other close paths dropped the socket immediately. For a direct connection this is safe, but for a proxied pool the socket is tunneled over a shared SpdySession. Destroying such a socket can synchronously drain the SpdySession and fail the session’s pending ConnectJobs, which reentrantly invokes TransportClientSocketPool::OnConnectJobComplete(). That reentrant call can call RemoveGroup and erase the very Group the outer operation (e.g. CleanupIdleSockets) is mid-iteration over, leaving the outer frame holding a dangling Group*/iterator — a use-after-free, exercised in the regression test by overflowing the SpdySession send window (INT32_MAX) when discarding a queued DATA frame on stream cancellation, which drains the session during CleanupIdleSockets and erases the Group while it is still on the stack.

The patch introduces QueueDestroySocket(std::unique_ptr<StreamSocket>) and DestroyQueuedSockets(). For a direct proxy chain (GetProxyChain().is_direct()), QueueDestroySocket returns immediately and the caller’s unique_ptr destroys the socket synchronously as before. For a non-direct (proxied) chain, the socket is moved into the new sockets_queued_for_destruction_ vector and a task is posted (on first enqueue) to run DestroyQueuedSockets later, deferring the drain of the shared session until the pool is no longer mid-operation. Every inline destruction site is rewritten to call QueueDestroySocket(std::move(...)) instead of letting the unique_ptr drop — in CleanupIdleSockets (idle-socket close loop), the socket-release path (replacing socket.reset()), the close-idle-socket path, and CloseOneIdleSocket. CloseIdleSockets additionally calls DestroyQueuedSockets() after cleanup to preserve its synchronous teardown contract. DestroyQueuedSockets swaps the queue into a local vector before destroying, so a socket destructor that reentrantly calls QueueDestroySocket is handled safely.

Key insight
In a proxied pool, destroying a tunneled socket inline synchronously drains the shared SpdySession and reentrantly fails ConnectJobs, which can erase the very Group the pool is still iterating — so the fix defers proxied-socket destruction to a posted task instead of running it mid-operation.

Attack Path

  1. Establish a tunnel Through an HTTPS (H2) proxy, a tunneled socket is established in a group, riding a shared SpdySession (proxied pool, GetProxyChain().is_direct() is false).
  2. Wedge the session write queue A DATA frame is queued on the tunnel stream while the session’s in-flight write is occupied (SETTINGS ACK kept in flight), so the frame stays in SpdySession::write_queue_. (Web-reachability of the precise frame timing is inferred; the diff proves it only via the regression test.)
  3. Arm the overflow A session WINDOW_UPDATE raises the send window to INT32_MAX; a start then cancel of a second same-group request leaves an unassigned pending ConnectJob, and unexpected tunnel data makes the idle socket unusable.
  4. Trigger cleanup A new socket request runs CleanupIdleSockets, which destroys the unusable idle socket inline.
  5. Reentrant drain Destroying the proxied socket discards the queued DATA frame, overflowing the send window past INT32_MAX and synchronously draining the SpdySession; this fails the pending ConnectJob and reentrantly calls OnConnectJobComplete, which RemoveGroups and erases the Group.
  6. Use-after-free Control returns into CleanupIdleSockets, still holding a pointer/iterator into the now-freed Group, yielding a use-after-free in the network service.

Impact Assessment

This is a use-after-free in the network-service process, triggered by reentrant mutation of TransportClientSocketPool::group_map_ while the pool is mid-operation. It requires the connection to go through a proxy (!GetProxyChain().is_direct()) over a shared SpdySession, and the attacker must arrange the specific session state (wedged write queue + window overflow on a cancelled stream) that makes destroying an idle/released socket drain the session synchronously; the diff substantiates this via its regression test rather than a bare web PoC, so direct unauthenticated web-reachability is (inferred). The primitive is a stale Group*/iterator access after RemoveGroup, i.e. use-after-free of pool-controlled heap state; whether that is escalated beyond a crash is not shown by the diff. Direct (non-proxied) connections are unaffected because QueueDestroySocket destroys them synchronously as before.

Changed Functions

FunctionChangeNotes
while
net/socket/transport_client_socket_pool.cc
modified
if
net/socket/transport_client_socket_pool.cc
modified

Files Changed

  • net/socket/transport_client_socket_pool.h
  • net/socket/transport_client_socket_pool_unittest.cc
  • net/socket/transport_client_socket_pool.cc

Audit Directions

  • Other inline socket destruction in pools
    Search transport_client_socket_pool.cc and sibling pools for remaining socket.reset() / unique_ptr drops of a StreamSocket that occur while iterating group_map_ or a Group’s socket lists.
  • Reentrancy via shared-session drain
    Audit any operation whose inner step can synchronously fail ConnectJobs or run OnConnectJobComplete/RemoveGroup (SPDY/QUIC proxy sockets, WebSocket-over-H2) for stale Group*/iterator use after the callback returns.
  • Destructor-driven callbacks
    Look for StreamSocket/SpdyProxyClientSocket destructors that perform synchronous session teardown and callback dispatch; those are the engines that turn an inline reset() into reentrancy.
  • Queue-drain safety
    Confirm every new QueueDestroySocket caller tolerates the socket outliving the current frame (notably CloseIdleSockets’s explicit DestroyQueuedSockets() and any DCHECK_EQ(0u, idle_socket_count_) ordering assumptions).
From 7dddd41a3f94cdd49beb6c8a53977d04c91368d2 Mon Sep 17 00:00:00 2001
From: Kenichi Ishibashi <[email protected]>
Date: Tue, 06 Oct 2026 22:35:20 -0700
Subject: [PATCH] net: Defer proxied StreamSocket destruction in TransportClientSocketPool

Destroying a tunneled StreamSocket (e.g., a SpdyProxyClientSocket) can
synchronously drain the underlying SpdySession and fail pending
ConnectJobs in the pool. This reentrantly invokes
TransportClientSocketPool::OnConnectJobComplete(), which can modify or
remove Group entries in `group_map_` while the pool is mid-operation.

To avoid reentrancy when closing sockets in a proxied pool, add
`QueueDestroySocket()`, which queues closed `StreamSocket`s into
`sockets_queued_for_destruction_` and posts a task to destroy them
asynchronously when `!GetProxyChain().is_direct()`. Direct connections
continue to be destroyed synchronously.

Bug: 568826881
Change-Id: Ied5765d08ea45050d35f4c746aa84a3c5f66e1ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8507984
Commit-Queue: Kenichi Ishibashi <[email protected]>
Reviewed-by: mmenke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1713084}
---
diff --git a/net/socket/transport_client_socket_pool.cc b/net/socket/transport_client_socket_pool.cc
index 9257bef..ade1f6b 100644
--- a/net/socket/transport_client_socket_pool.cc
+++ b/net/socket/transport_client_socket_pool.cc
@@ -460,8 +460,7 @@
     // group. Close idle sockets until we have capacity (or there are none
     // left outside the current group).
     while (idle_socket_count_ > 0) {
-      bool closed = CloseOneIdleSocketExceptInGroup(group);
-      if (!closed) {
+      if (!CloseOneIdleSocketExceptInGroup(group)) {
         break;
       }
       // We want to know if a new allocation could succeed, if not we must
@@ -549,6 +548,7 @@
       it->socket->NetLog().AddEventWithStringParams(
           NetLogEventType::SOCKET_POOL_CLOSING_SOCKET, "reason",
           net_log_reason_utf8);
+      QueueDestroySocket(std::move(it->socket));
       DecrementIdleCount();
       it = idle_sockets->erase(it);
       continue;
@@ -677,6 +677,7 @@
 void TransportClientSocketPool::CloseIdleSockets(
     const char* net_log_reason_utf8) {
   CleanupIdleSockets(true, net_log_reason_utf8);
+  DestroyQueuedSockets();
   DCHECK_EQ(0u, idle_socket_count_);
 }
 
@@ -999,6 +1000,7 @@
       idle_socket_it->socket->NetLog().AddEventWithStringParams(
           NetLogEventType::SOCKET_POOL_CLOSING_SOCKET, "reason",
           reason_for_closing_socket);
+      QueueDestroySocket(std::move(idle_socket_it->socket));
       idle_socket_it = group->mutable_idle_sockets()->erase(idle_socket_it);
       DecrementIdleCount();
       UpdateExpandabilityAfterRelease();
@@ -1079,7 +1081,7 @@
         not_reusable_reason);
     if (group->IsEmpty())
       RemoveGroup(i);
-    socket.reset();
+    QueueDestroySocket(std::move(socket));
   }
 
   CheckForStalledSocketGroups();
@@ -1315,6 +1317,7 @@
     std::list<IdleSocket>* idle_sockets = group->mutable_idle_sockets();
 
     if (!idle_sockets->empty()) {
+      QueueDestroySocket(std::move(idle_sockets->front().socket));
       idle_sockets->pop_front();
       DecrementIdleCount();
       if (group->IsEmpty())
@@ -1507,6 +1510,27 @@
   return ++it;
 }
 
+void TransportClientSocketPool::QueueDestroySocket(
+    std::unique_ptr<StreamSocket> socket) {
+  if (GetProxyChain().is_direct()) {
+    return;
+  }
+  if (sockets_queued_for_destruction_.empty()) {
+    base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&TransportClientSocketPool::DestroyQueuedSockets,
+                       weak_factory_.GetWeakPtr()));
+  }
+  sockets_queued_for_destruction_.emplace_back(std::move(socket));
+}
+
+void TransportClientSocketPool::DestroyQueuedSockets() {
+  // Move into a local vector before destroying elements in case a socket's
+  // destructor reentrantly calls QueueDestroySocket().
+  std::vector<std::unique_ptr<StreamSocket>> sockets =
+      std::move(sockets_queued_for_destruction_);
+}
+
 void TransportClientSocketPool::OnPreconnectConnectJobComplete(
     PreconnectCompletionCallback callback,
     const GroupId& group_id,
diff --git a/net/socket/transport_client_socket_pool.h b/net/socket/transport_client_socket_pool.h
index 5436395f..9273811 100644
--- a/net/socket/transport_client_socket_pool.h
+++ b/net/socket/transport_client_socket_pool.h
@@ -772,6 +772,13 @@
                                   const base::TimeTicks& now,
                                   const char* net_log_reason_utf8);
 
+  // Destroys |socket| immediately for direct connections, or queues it for
+  // asynchronous destruction when using a proxy, where destroying a tunneled
+  // socket can synchronously drain a shared session and reentrantly complete
+  // ConnectJobs in |this|.
+  void QueueDestroySocket(std::unique_ptr<StreamSocket> socket);
+  void DestroyQueuedSockets();
+
   // Called when a preconnect connect job completes.
   void OnPreconnectConnectJobComplete(
       PreconnectCompletionCallback callback,
@@ -815,6 +822,8 @@
 
   const raw_ptr<SSLClientContext> ssl_client_context_;
 
+  std::vector<std::unique_ptr<StreamSocket>> sockets_queued_for_destruction_;
+
 #if DCHECK_IS_ON()
   // Reentrancy guard for RequestSocketInternal().
   bool request_in_process_ = false;
diff --git a/net/socket/transport_client_socket_pool_unittest.cc b/net/socket/transport_client_socket_pool_unittest.cc
index 4be6938..19548e954 100644
--- a/net/socket/transport_client_socket_pool_unittest.cc
+++ b/net/socket/transport_client_socket_pool_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "net/socket/transport_client_socket_pool.h"
 
+#include <limits>
 #include <memory>
 #include <optional>
 #include <utility>
@@ -2520,6 +2521,146 @@
       result.Get<std::unique_ptr<ClientSocketHandle>>()->is_initialized());
 }
 
+// Regression test for crbug.com/568826881: Destroying an unusable idle socket
+// during CleanupIdleSockets() can synchronously drain the underlying
+// SpdySession (e.g., on send-window overflow when discarding a queued DATA
+// frame), failing a pending ConnectJob in the same group and erasing the Group
+// reentrantly while CleanupIdleSockets() still holds a pointer/iterator to it.
+TEST_P(TransportClientSocketPoolTest,
+       CleanupIdleSocketsReentrantGroupDestruction) {
+  const url::SchemeHostPort kEndpoint(url::kHttpsScheme, "host.test", 443);
+
+  session_deps_.host_resolver->set_synchronous_mode(true);
+
+  TransportClientSocketPool pool(
+      kMaxSockets, kMaxSocketsPerGroup, kUnusedIdleSocketTimeout,
+      ProxyUriToProxyChain("https://proxy.test",
+                           /*default_scheme=*/ProxyServer::SCHEME_HTTP),
+      /*is_for_websockets=*/false, tagging_common_connect_job_params_.get());
+
+  SpdyTestUtil spdy_util;
+  spdy::SpdySerializedFrame connect(spdy_util.ConstructSpdyConnect(
+      base::span<const std::string_view>(), 1,
+      HttpProxyConnectJob::kH2QuicTunnelPriority,
+      HostPortPair::FromSchemeHostPort(kEndpoint)));
+
+  spdy::SettingsMap settings_map;
+  settings_map[spdy::SETTINGS_MAX_CONCURRENT_STREAMS] = 1;
+  spdy::SpdySerializedFrame settings(
+      spdy_util.ConstructSpdySettings(settings_map));
+  spdy::SpdySerializedFrame settings_ack(spdy_util.ConstructSpdySettingsAck());
+  spdy::SpdySerializedFrame connect_reply(
+      spdy_util.ConstructSpdyGetReply(base::span<const std::string_view>(), 1));
+
+  // Initial session send window is 65535. After queueing a 10-byte DATA frame
+  // on stream 1, the session send window is 65525. Raising it by
+  // (INT32_MAX - 65525) sets it to INT32_MAX, so discarding the 10-byte DATA
+  // frame on stream cancellation overflows INT32_MAX and synchronously drains
+  // the SpdySession.
+  constexpr std::string_view kTunnelWriteData = "0123456789";
+  spdy::SpdySerializedFrame window_update(spdy_util.ConstructSpdyWindowUpdate(
+      0, std::numeric_limits<int32_t>::max() - 65535 +
+             static_cast<uint32_t>(kTunnelWriteData.size())));
+  spdy::SpdySerializedFrame unexpected_data(
+      spdy_util.ConstructSpdyDataFrame(1, "x", /*fin=*/false));
+
+  MockWrite writes[] = {
+      CreateMockWrite(connect, 0, ASYNC),
+      // Keep the SETTINGS ACK in flight (`SpdySession::in_flight_write_`) by
+      // giving it sequence number 7 (after the pause at sequence number 6), so
+      // that subsequent stream DATA frames remain in
+      // `SpdySession::write_queue_`.
+      CreateMockWrite(settings_ack, 7, ASYNC),
+  };
+
+  MockRead reads[] = {
+      CreateMockRead(settings, 1, ASYNC),
+      CreateMockRead(connect_reply, 2, ASYNC),
+      MockRead(ASYNC, ERR_IO_PENDING, 3),
+      CreateMockRead(window_update, 4, ASYNC),
+      CreateMockRead(unexpected_data, 5, ASYNC),
+      MockRead(ASYNC, ERR_IO_PENDING, 6),
+  };
+
+  SequencedSocketData socket_data(MockConnect(SYNCHRONOUS, OK), reads, writes);
+  tagging_client_socket_factory_.AddSocketDataProvider(&socket_data);
+  // SSL data for the connection to the HTTPS proxy.
+  SSLSocketDataProvider ssl_proxy_data(SYNCHRONOUS, OK);
+  ssl_proxy_data.next_proto = NextProto::kProtoHTTP2;
+  tagging_client_socket_factory_.AddSSLSocketDataProvider(&ssl_proxy_data);
+  // SSL data for the inner tunnel handshake to `kEndpoint`.
+  SSLSocketDataProvider ssl_tunnel_data(SYNCHRONOUS, OK);
+  tagging_client_socket_factory_.AddSSLSocketDataProvider(&ssl_tunnel_data);
+
+  scoped_refptr<ClientSocketPool::SocketParams> socket_params =
+      base::MakeRefCounted<ClientSocketPool::SocketParams>(
+          /*allowed_bad_certs=*/std::vector<SSLConfig::CertAndStatus>());
+
+  ClientSocketPool::GroupId group_id(
+      kEndpoint, PrivacyMode::PRIVACY_MODE_DISABLED, NetworkAnonymizationKey(),
+      SecureDnsPolicy::kAllow, /*disable_cert_network_fetches=*/false,
+      handles::kInvalidNetworkHandle);
+
+  // 1. Establish the tunnel socket (stream 1).
+  TestCompletionCallback callback1;
+  ClientSocketHandle handle1;
+  int rv1 = handle1.Init(
+      group_id, socket_params, TRAFFIC_ANNOTATION_FOR_TESTS, HIGHEST,
+      SocketTag(), ClientSocketPool::RespectLimits::ENABLED,
+      callback1.callback(), ClientSocketPool::ProxyAuthCallback(), &pool,
+      NetLogWithSource());
+  ASSERT_THAT(callback1.GetResult(rv1), IsOk());
+  ASSERT_TRUE(handle1.socket());
+
+  // 2. Queue a DATA frame on stream 1. Because `in_flight_write_` is wedged on
+  // the SETTINGS ACK (seq 4), this frame stays in `SpdySession::write_queue_`.
+  auto write_buf =
+      base::MakeRefCounted<StringIOBuffer>(std::string(kTunnelWriteData));
+  TestCompletionCallback write_callback;
+  int write_rv = handle1.socket()->Write(write_buf.get(), write_buf->size(),
+                                         write_callback.callback(),
+                                         TRAFFIC_ANNOTATION_FOR_TESTS);
+  EXPECT_THAT(write_rv, IsError(ERR_IO_PENDING));
+
+  // 3. Start a second request in the same group while `handle1` is active. Its
+  // ConnectJob queues a `SpdyStreamRequest` behind stream 1 due to
+  // `SETTINGS_MAX_CONCURRENT_STREAMS = 1`. Cancel the request without
+  // canceling the ConnectJob so the group has an unassigned ConnectJob.
+  TestCompletionCallback callback2;
+  ClientSocketHandle handle2;
+  int rv2 = handle2.Init(
+      group_id, socket_params, TRAFFIC_ANNOTATION_FOR_TESTS, LOW, SocketTag(),
+      ClientSocketPool::RespectLimits::ENABLED, callback2.callback(),
+      ClientSocketPool::ProxyAuthCallback(), &pool, NetLogWithSource());
+  ASSERT_THAT(rv2, IsError(ERR_IO_PENDING));
+  handle2.Reset();
+
+  // 4. Return `handle1`'s socket to the pool as an idle socket.
+  handle1.Reset();
+  EXPECT_EQ(1u, pool.IdleSocketCount());
+  EXPECT_EQ(1u, pool.NumConnectJobsInGroupForTesting(group_id));
+
+  // 5. Deliver the session WINDOW_UPDATE (arming the send window to INT32_MAX)
+  // and 1 byte of unexpected tunnel data (making the idle socket unusable).
+  socket_data.Resume();
+  socket_data.RunUntilPaused();
+
+  // 6. Requesting a socket runs `CleanupIdleSockets(false)`. Without deferred
+  // socket destruction, destroying the unusable idle socket overflows the
+  // session send window and drains the SpdySession synchronously, failing the
+  // pending ConnectJob and erasing the Group while CleanupIdleSockets() is
+  // still on the stack.
+  TestCompletionCallback callback3;
+  ClientSocketHandle handle3;
+  int rv3 = handle3.Init(
+      group_id, socket_params, TRAFFIC_ANNOTATION_FOR_TESTS, LOW, SocketTag(),
+      ClientSocketPool::RespectLimits::ENABLED, callback3.callback(),
+      ClientSocketPool::ProxyAuthCallback(), &pool, NetLogWithSource());
+  EXPECT_THAT(callback3.GetResult(rv3), IsError(ERR_HTTP2_PROTOCOL_ERROR));
+  EXPECT_EQ(0u, pool.IdleSocketCount());
+  EXPECT_FALSE(pool.HasGroupForTesting(group_id));
+}
+
 // Test that SocketTag passed into TransportClientSocketPool is applied to
 // returned sockets.
 #if BUILDFLAG(IS_ANDROID)
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/net/socket/transport_client_socket_pool_unittest.cc b/net/socket/transport_client_socket_pool_unittest.cc
index 4be6938..19548e954 100644
--- a/net/socket/transport_client_socket_pool_unittest.cc
+++ b/net/socket/transport_client_socket_pool_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "net/socket/transport_client_socket_pool.h"
 
+#include <limits>
 #include <memory>
 #include <optional>
 #include <utility>
@@ -2520,6 +2521,146 @@
       result.Get<std::unique_ptr<ClientSocketHandle>>()->is_initialized());
 }
 
+// Regression test for crbug.com/568826881: Destroying an unusable idle socket
+// during CleanupIdleSockets() can synchronously drain the underlying
+// SpdySession (e.g., on send-window overflow when discarding a queued DATA
+// frame), failing a pending ConnectJob in the same group and erasing the Group
+// reentrantly while CleanupIdleSockets() still holds a pointer/iterator to it.
+TEST_P(TransportClientSocketPoolTest,
+       CleanupIdleSocketsReentrantGroupDestruction) {
+  const url::SchemeHostPort kEndpoint(url::kHttpsScheme, "host.test", 443);
+
+  session_deps_.host_resolver->set_synchronous_mode(true);
+
+  TransportClientSocketPool pool(
+      kMaxSockets, kMaxSocketsPerGroup, kUnusedIdleSocketTimeout,
+      ProxyUriToProxyChain("https://proxy.test",
+                           /*default_scheme=*/ProxyServer::SCHEME_HTTP),
+      /*is_for_websockets=*/false, tagging_common_connect_job_params_.get());
+
+  SpdyTestUtil spdy_util;
+  spdy::SpdySerializedFrame connect(spdy_util.ConstructSpdyConnect(
+      base::span<const std::string_view>(), 1,
+      HttpProxyConnectJob::kH2QuicTunnelPriority,
+      HostPortPair::FromSchemeHostPort(kEndpoint)));
+
+  spdy::SettingsMap settings_map;
+  settings_map[spdy::SETTINGS_MAX_CONCURRENT_STREAMS] = 1;
+  spdy::SpdySerializedFrame settings(
+      spdy_util.ConstructSpdySettings(settings_map));
+  spdy::SpdySerializedFrame settings_ack(spdy_util.ConstructSpdySettingsAck());
+  spdy::SpdySerializedFrame connect_reply(
+      spdy_util.ConstructSpdyGetReply(base::span<const std::string_view>(), 1));
+
+  // Initial session send window is 65535. After queueing a 10-byte DATA frame
+  // on stream 1, the session send window is 65525. Raising it by
+  // (INT32_MAX - 65525) sets it to INT32_MAX, so discarding the 10-byte DATA
+  // frame on stream cancellation overflows INT32_MAX and synchronously drains
+  // the SpdySession.
+  constexpr std::string_view kTunnelWriteData = "0123456789";
+  spdy::SpdySerializedFrame window_update(spdy_util.ConstructSpdyWindowUpdate(
+      0, std::numeric_limits<int32_t>::max() - 65535 +
+             static_cast<uint32_t>(kTunnelWriteData.size())));
+  spdy::SpdySerializedFrame unexpected_data(
+      spdy_util.ConstructSpdyDataFrame(1, "x", /*fin=*/false));
+
+  MockWrite writes[] = {
+      CreateMockWrite(connect, 0, ASYNC),
+      // Keep the SETTINGS ACK in flight (`SpdySession::in_flight_write_`) by
+      // giving it sequence number 7 (after the pause at sequence number 6), so
+      // that subsequent stream DATA frames remain in
+      // `SpdySession::write_queue_`.
+      CreateMockWrite(settings_ack, 7, ASYNC),
+  };
+
+  MockRead reads[] = {
+      CreateMockRead(settings, 1, ASYNC),
+      CreateMockRead(connect_reply, 2, ASYNC),
+      MockRead(ASYNC, ERR_IO_PENDING, 3),
+      CreateMockRead(window_update, 4, ASYNC),
+      CreateMockRead(unexpected_data, 5, ASYNC),
+      MockRead(ASYNC, ERR_IO_PENDING, 6),
+  };
+
+  SequencedSocketData socket_data(MockConnect(SYNCHRONOUS, OK), reads, writes);
+  tagging_client_socket_factory_.AddSocketDataProvider(&socket_data);
+  // SSL data for the connection to the HTTPS proxy.
+  SSLSocketDataProvider ssl_proxy_data(SYNCHRONOUS, OK);
+  ssl_proxy_data.next_proto = NextProto::kProtoHTTP2;
+  tagging_client_socket_factory_.AddSSLSocketDataProvider(&ssl_proxy_data);
+  // SSL data for the inner tunnel handshake to `kEndpoint`.
+  SSLSocketDataProvider ssl_tunnel_data(SYNCHRONOUS, OK);
+  tagging_client_socket_factory_.AddSSLSocketDataProvider(&ssl_tunnel_data);
+
+  scoped_refptr<ClientSocketPool::SocketParams> socket_params =
+      base::MakeRefCounted<ClientSocketPool::SocketParams>(
+          /*allowed_bad_certs=*/std::vector<SSLConfig::CertAndStatus>());
+
+  ClientSocketPool::GroupId group_id(
+      kEndpoint, PrivacyMode::PRIVACY_MODE_DISABLED, NetworkAnonymizationKey(),
+      SecureDnsPolicy::kAllow, /*disable_cert_network_fetches=*/false,
+      handles::kInvalidNetworkHandle);
+
+  // 1. Establish the tunnel socket (stream 1).
+  TestCompletionCallback callback1;
+  ClientSocketHandle handle1;
+  int rv1 = handle1.Init(
+      group_id, socket_params, TRAFFIC_ANNOTATION_FOR_TESTS, HIGHEST,
+      SocketTag(), ClientSocketPool::RespectLimits::ENABLED,
+      callback1.callback(), ClientSocketPool::ProxyAuthCallback(), &pool,
+      NetLogWithSource());
+  ASSERT_THAT(callback1.GetResult(rv1), IsOk());
+  ASSERT_TRUE(handle1.socket());
+
+  // 2. Queue a DATA frame on stream 1. Because `in_flight_write_` is wedged on
+  // the SETTINGS ACK (seq 4), this frame stays in `SpdySession::write_queue_`.
+  auto write_buf =
+      base::MakeRefCounted<StringIOBuffer>(std::string(kTunnelWriteData));
+  TestCompletionCallback write_callback;
+  int write_rv = handle1.socket()->Write(write_buf.get(), write_buf->size(),
+                                         write_callback.callback(),
+                                         TRAFFIC_ANNOTATION_FOR_TESTS);
+  EXPECT_THAT(write_rv, IsError(ERR_IO_PENDING));
+
+  // 3. Start a second request in the same group while `handle1` is active. Its
+  // ConnectJob queues a `SpdyStreamRequest` behind stream 1 due to
+  // `SETTINGS_MAX_CONCURRENT_STREAMS = 1`. Cancel the request without
+  // canceling the ConnectJob so the group has an unassigned ConnectJob.
+  TestCompletionCallback callback2;
+  ClientSocketHandle handle2;
+  int rv2 = handle2.Init(
+      group_id, socket_params, TRAFFIC_ANNOTATION_FOR_TESTS, LOW, SocketTag(),
+      ClientSocketPool::RespectLimits::ENABLED, callback2.callback(),
+      ClientSocketPool::ProxyAuthCallback(), &pool, NetLogWithSource());
+  ASSERT_THAT(rv2, IsError(ERR_IO_PENDING));
+  handle2.Reset();
+
+  // 4. Return `handle1`'s socket to the pool as an idle socket.
+  handle1.Reset();
+  EXPECT_EQ(1u, pool.IdleSocketCount());
+  EXPECT_EQ(1u, pool.NumConnectJobsInGroupForTesting(group_id));
+
+  // 5. Deliver the session WINDOW_UPDATE (arming the send window to INT32_MAX)
+  // and 1 byte of unexpected tunnel data (making the idle socket unusable).
+  socket_data.Resume();
+  socket_data.RunUntilPaused();
+
+  // 6. Requesting a socket runs `CleanupIdleSockets(false)`. Without deferred
+  // socket destruction, destroying the unusable idle socket overflows the
+  // session send window and drains the SpdySession synchronously, failing the
+  // pending ConnectJob and erasing the Group while CleanupIdleSockets() is
+  // still on the stack.
+  TestCompletionCallback callback3;
+  ClientSocketHandle handle3;
+  int rv3 = handle3.Init(
+      group_id, socket_params, TRAFFIC_ANNOTATION_FOR_TESTS, LOW, SocketTag(),
+      ClientSocketPool::RespectLimits::ENABLED, callback3.callback(),
+      ClientSocketPool::ProxyAuthCallback(), &pool, NetLogWithSource());
+  EXPECT_THAT(callback3.GetResult(rv3), IsError(ERR_HTTP2_PROTOCOL_ERROR));
+  EXPECT_EQ(0u, pool.IdleSocketCount());
+  EXPECT_FALSE(pool.HasGroupForTesting(group_id));
+}
+
 // Test that SocketTag passed into TransportClientSocketPool is applied to
 // returned sockets.
 #if BUILDFLAG(IS_ANDROID)
Loading diff…

Original Bug Report

The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.