[M90-LTS] Protect candidate better from garbage collection during neg…

…otiation. Includes a test that was reliably observed to produce an UAF on Linux when compiled with ASAN before the fix. (cherry picked from commit 654536e) Bug: chromium:1230767 Change-Id: I02dd29332a6d00790dcace41b6584b96413ef6f4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3057049 Commit-Queue: Harald Alvestrand <hta@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#910244} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3102948 Reviewed-by: Artem Sumaneev <asumaneev@google.com> Owners-Override: Artem Sumaneev <asumaneev@google.com> Commit-Queue: Roger Felipe Zanoni da Silva <rzanoni@google.com> Cr-Commit-Position: refs/branch-heads/4430@{#1570} Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}
chromium · Aug 20, 2021 · 010a318 · 010a318
1 parent d8f7a22
commit 010a318
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 2 deletions.
diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc b/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc
@@ -1623,7 +1623,8 @@ void RTCPeerConnectionHandler::AddICECandidate(
       std::move(native_candidate),
       [pc = native_peer_connection_, task_runner = task_runner_,
        handler_weak_ptr = weak_factory_.GetWeakPtr(),
-       tracker_weak_ptr = peer_connection_tracker_, candidate,
+       tracker_weak_ptr = peer_connection_tracker_,
+       persistent_candidate = WrapCrossThreadPersistent(candidate),
        persistent_request = WrapCrossThreadPersistent(request),
        callback_on_task_runner =
            std::move(callback_on_task_runner)](webrtc::RTCError result) {
@@ -1652,7 +1653,7 @@ void RTCPeerConnectionHandler::AddICECandidate(
                 std::move(current_local_description),
                 std::move(pending_remote_description),
                 std::move(current_remote_description),
-                WrapCrossThreadPersistent(candidate), std::move(result),
+                std::move(persistent_candidate), std::move(result),
                 std::move(persistent_request)));
       });
 }

diff --git a/third_party/blink/web_tests/fast/peerconnection/poc-123067.html b/third_party/blink/web_tests/fast/peerconnection/poc-123067.html
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<html>
+
+  <head>
+    <script src="../../resources/testharness.js"></script>
+    <script src="../../resources/testharnessreport.js"></script>
+    <script src="../../resources/gc.js"></script>
+  </head>
+  <body>
+  <script>
+    'use strict';
+  promise_test(async t => {
+    const var_caller_1 = new RTCPeerConnection();
+    const var_callee_1 = new RTCPeerConnection();
+    var_caller_1.addTransceiver('audio');
+    const var_prom_1 = new Promise(resolve => {
+      var_caller_1.onicecandidate = e => resolve(e.candidate);
+    });
+    await var_caller_1.setLocalDescription(await var_caller_1.createOffer());
+    await var_callee_1.setRemoteDescription(var_caller_1.localDescription);
+    const candidate = await var_prom_1;
+    var arrProm = [];
+    gc();
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.setLocalDescription().then(() => {
+    })
+    var_callee_1.addIceCandidate(candidate).then(() => {
+    })
+    await Promise.all(arrProm);
+  }, 'Running this script does not cause an UAF');
+  </script>
+</head>
+
+<body></body>
+
+</html>