diff options
author | Henrik Boström <hbos@chromium.org> | 2023-03-14 13:07:19 +0000 |
---|---|---|
committer | Michael Brüning <michael.bruning@qt.io> | 2023-03-24 10:06:31 +0000 |
commit | 17fe88a2d7da7d6150fdcbcb764d4f029da4f3f7 (patch) | |
tree | 5ed49605f61bcf028cb509c8f9d341c5311bf9b8 | |
parent | 6e261dcd44f8bc097c0eac394edf2c29365aa2e2 (diff) | |
download | qtwebengine-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>
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, |