From 097169de52a7ab59a735eaa8a45a55887f87920d Mon Sep 17 00:00:00 2001 From: Kenichi Ishibashi Date: Wed, 12 Apr 2023 08:58:55 +0000 Subject: [Backport] Security bug 1428820 (1/3) Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/4393905: Remove the second WeakPtrFactory from SpdyProxyClientSocket It was introduced [1] to work around an old issue that wouldn't happen any more since we store a write callback in the class. Instead of having the second WeakPtrFactory and moving the callback, we can just keep it until RunWriteCallback() is called. This is a speculative fix for the linked bug. [1] https://codereview.chromium.org/338583003/ Bug: 1428820 Change-Id: I0b5af2675b68188e208c2ecd42293251b2722b28 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4393905 Commit-Queue: Kenichi Ishibashi Cr-Commit-Position: refs/heads/main@{#1125216} (cherry picked from commit 01b25615896b911e21103dd381fafc1f85886d91) Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/474645 Reviewed-by: Michal Klocek --- chromium/net/spdy/spdy_proxy_client_socket.cc | 10 ++++------ chromium/net/spdy/spdy_proxy_client_socket.h | 10 ++-------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/chromium/net/spdy/spdy_proxy_client_socket.cc b/chromium/net/spdy/spdy_proxy_client_socket.cc index 5984ecedb3a..173fc5cdbb6 100644 --- a/chromium/net/spdy/spdy_proxy_client_socket.cc +++ b/chromium/net/spdy/spdy_proxy_client_socket.cc @@ -123,7 +123,6 @@ void SpdyProxyClientSocket::Disconnect() { write_buffer_len_ = 0; write_callback_.Reset(); - write_callback_weak_factory_.InvalidateWeakPtrs(); next_state_ = STATE_DISCONNECTED; @@ -278,9 +277,9 @@ int SpdyProxyClientSocket::GetLocalAddress(IPEndPoint* address) const { return spdy_stream_->GetLocalAddress(address); } -void SpdyProxyClientSocket::RunWriteCallback(CompletionOnceCallback callback, - int result) const { - std::move(callback).Run(result); +void SpdyProxyClientSocket::RunWriteCallback(int result) { + CHECK(write_callback_); + std::move(write_callback_).Run(result); if (end_stream_state_ == EndStreamState::kEndStreamReceived) { base::ThreadTaskRunnerHandle::Get()->PostTask( @@ -517,8 +516,7 @@ void SpdyProxyClientSocket::OnDataSent() { // stream's write callback chain to unwind (see crbug.com/355511). base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(&SpdyProxyClientSocket::RunWriteCallback, - write_callback_weak_factory_.GetWeakPtr(), - std::move(write_callback_), rv)); + weak_factory_.GetWeakPtr(), rv)); } void SpdyProxyClientSocket::OnTrailers(const spdy::Http2HeaderBlock& trailers) { diff --git a/chromium/net/spdy/spdy_proxy_client_socket.h b/chromium/net/spdy/spdy_proxy_client_socket.h index cbb961a68c9..98b9e74427f 100644 --- a/chromium/net/spdy/spdy_proxy_client_socket.h +++ b/chromium/net/spdy/spdy_proxy_client_socket.h @@ -121,9 +121,9 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket, STATE_CLOSED }; - // Calls |callback.Run(result)|. Used to run a callback posted to the + // Calls `write_callback_(result)`. Used to run a callback posted to the // message loop. - void RunWriteCallback(CompletionOnceCallback callback, int result) const; + void RunWriteCallback(int result); void OnIOComplete(int result); @@ -195,13 +195,7 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket, }; EndStreamState end_stream_state_ = EndStreamState::kNone; - // The default weak pointer factory. base::WeakPtrFactory weak_factory_{this}; - - // Only used for posting write callbacks. Weak pointers created by this - // factory are invalidated in Disconnect(). - base::WeakPtrFactory write_callback_weak_factory_{ - this}; }; } // namespace net -- cgit v1.2.1