[Backport] CVE-2024-1059: Use after free in WebRTC

Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5210359:
[RTCPeerConnection] Exit early from RTCPeerConnectionHandler

For certain operations that require a live client
(i.e., RTCPeerConnection, which is garbage collected),
PeerConnectionHandler keeps a pointer to the client on the stack
to prevent garbage collection.

In some cases, the client may have already been garbage collected
(the client is null). In that case, there is no point in doing the
operation and it should exit early to avoid UAF/crashes.

This CL adds early exit to the cases that do not already have it.

Bug: 1514777
Change-Id: I27e9541cfaa74d978799c03e2832a0980f9e5710
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5210359
Reviewed-by: Tomas Gunnarsson <tommi@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1248826}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/537379
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

1 files changed, 13 insertions, 4 deletions

diff --git a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc
index 7781c4083dc..abd1bdd6634 100644
--- a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc
+++ b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc
@@ -1110,14 +1110,18 @@ bool RTCPeerConnectionHandler::Initialize(
     const MediaConstraints& options,
     WebLocalFrame* frame) {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
-  DCHECK(frame);
-  frame_ = frame;
 
   CHECK(!initialize_called_);
   initialize_called_ = true;
 
   // Prevent garbage collection of client_ during processing.
   auto* client_on_stack = client_;
+  if (!client_on_stack) {
+    return false;
+  }
+
+  DCHECK(frame);
+  frame_ = frame;
   // TODO(crbug.com/787254): Evaluate the need for passing weak ptr since
   // PeerConnectionTracker is now leaky with base::LazyInstance.
   peer_connection_tracker_ = PeerConnectionTracker::GetInstance()->AsWeakPtr();
@@ -2523,10 +2527,13 @@ void RTCPeerConnectionHandler::OnIceCandidate(const String& sdp,
                                               int sdp_mline_index,
                                               int component,
                                               int address_family) {
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
   // In order to ensure that the RTCPeerConnection is not garbage collected
   // from under the function, we keep a pointer to it on the stack.
   auto* client_on_stack = client_;
-  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+  if (!client_on_stack) {
+    return;
+  }
   TRACE_EVENT0("webrtc", "RTCPeerConnectionHandler::OnIceCandidateImpl");
   // This line can cause garbage collection.
   auto* platform_candidate = MakeGarbageCollected<RTCIceCandidatePlatform>(
@@ -2547,7 +2554,9 @@ void RTCPeerConnectionHandler::OnIceCandidate(const String& sdp,
       NOTREACHED();
     }
   }
-  if (!is_closed_)
+
+  client_on_stack = client_;
+  if (!is_closed_ && client_on_stack)
     client_on_stack->DidGenerateICECandidate(platform_candidate);
 }