summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHenrik Boström <hbos@chromium.org>2023-03-14 13:07:19 +0000
committerMichael Brüning <michael.bruning@qt.io>2023-03-24 10:06:31 +0000
commit17fe88a2d7da7d6150fdcbcb764d4f029da4f3f7 (patch)
tree5ed49605f61bcf028cb509c8f9d341c5311bf9b8
parent6e261dcd44f8bc097c0eac394edf2c29365aa2e2 (diff)
downloadqtwebengine-chromium-17fe88a2d7da7d6150fdcbcb764d4f029da4f3f7.tar.gz
[Backport] CVE-2023-1218: Use after free in WebRTC
Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/4291513: Shutdown RtpContributingSourceCache in Dispose(). The cache is an off-heap object, but it is owned by an on-heap object (RTCPeerConnection). Dispoing the owning object poisons memory owned by it, but the cache may have in-flight tasks (cache doing ClearCache in a delayed microtask). This CL adds a Shutdown() method to ensure the cache isn't doing anything in the next microtask after disposal. No reliable way to repro this has been found but the change should be safe so hoping we can land without tests. (cherry picked from commit 4d450ecd6ec7776c7505dcf7d2f04157ff3ba0eb) Bug: 1413628 Change-Id: I479aace9859f4c10cd75d4aa5a34808b4726299d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4247023 Commit-Queue: Henrik Boström <hbos@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1105653} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4291513 Reviewed-by: Achuith Bhandarkar <achuith@chromium.org> Owners-Override: Achuith Bhandarkar <achuith@chromium.org> Reviewed-by: Henrik Boström <hbos@chromium.org> Commit-Queue: Zakhar Voit <voit@google.com> Cr-Commit-Position: refs/branch-heads/5359@{#1404} Cr-Branched-From: 27d3765d341b09369006d030f83f582a29eb57ae-refs/heads/main@{#1058933} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/468219 Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r--chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc12
-rw-r--r--chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.cc4
-rw-r--r--chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.h4
3 files changed, 17 insertions, 3 deletions
diff --git a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
index 636aa342529..be9196ee034 100644
--- a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
+++ b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
@@ -893,12 +893,18 @@ RTCPeerConnection::~RTCPeerConnection() {
}
void RTCPeerConnection::Dispose() {
- // Promptly clears the handler
- // so that content/ doesn't access it in a lazy sweeping phase.
- // Other references to the handler use a weak pointer, preventing access.
+ // Promptly clears the handler so that content doesn't access it in a lazy
+ // sweeping phase. Other references to the handler use a weak pointer,
+ // preventing access.
if (peer_handler_) {
peer_handler_.reset();
}
+ // Memory owned by RTCPeerConnection must not be touched after Dispose().
+ // Shut down the cache to cancel any in-flight tasks that may otherwise have
+ // used the cache.
+ if (rtp_contributing_source_cache_.has_value()) {
+ rtp_contributing_source_cache_.value().Shutdown();
+ }
}
ScriptPromise RTCPeerConnection::createOffer(ScriptState* script_state,
diff --git a/chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.cc b/chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.cc
index a52ddd405eb..d4b21570e64 100644
--- a/chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.cc
+++ b/chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.cc
@@ -100,6 +100,10 @@ RtpContributingSourceCache::RtpContributingSourceCache(
DCHECK(worker_thread_runner_);
}
+void RtpContributingSourceCache::Shutdown() {
+ weak_factory_.InvalidateWeakPtrs();
+}
+
HeapVector<Member<RTCRtpSynchronizationSource>>
RtpContributingSourceCache::getSynchronizationSources(
ScriptState* script_state,
diff --git a/chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.h b/chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.h
index b8d78d80aee..827f5eb3524 100644
--- a/chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.h
+++ b/chromium/third_party/blink/renderer/modules/peerconnection/rtp_contributing_source_cache.h
@@ -43,6 +43,10 @@ class RtpContributingSourceCache {
RTCPeerConnection* pc,
scoped_refptr<base::SingleThreadTaskRunner> worker_thread_runner);
+ // When the owner of this object is Disposed(), this method must be called to
+ // cancel any in-flight tasks.
+ void Shutdown();
+
HeapVector<Member<RTCRtpSynchronizationSource>> getSynchronizationSources(
ScriptState* script_state,
ExceptionState& exception_state,