diff options
author | Harald Alvestrand <hta@chromium.org> | 2020-04-15 14:03:03 +0000 |
---|---|---|
committer | Michal Klocek <michal.klocek@qt.io> | 2020-06-11 09:05:34 +0000 |
commit | 8a53e97dba1ec531727914b0189db93d21f977b8 (patch) | |
tree | fbef579ae51491687d2487cfae6cf28958c7348d /chromium | |
parent | 04e8b821b3635e7d8b32853067894253028a2626 (diff) | |
download | qtwebengine-chromium-8a53e97dba1ec531727914b0189db93d21f977b8.tar.gz |
[Backport] CVE-2020-6467v5.12.9
Onstate handler is allowed to close a PeerConnection.
Bug: chromium:1068084
Change-Id: Icd3f70b6784ac22ef4e3bc1c99233f51145a917f
Reviewed-by: Michael BrĂ¼ning <michael.bruning@qt.io>
Diffstat (limited to 'chromium')
4 files changed, 35 insertions, 6 deletions
diff --git a/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.cc b/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.cc index 8a9329b9a81..1332ba89e93 100644 --- a/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.cc +++ b/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.cc @@ -1041,6 +1041,7 @@ RTCPeerConnectionHandler::RTCPeerConnectionHandler( initialize_called_(false), client_(client), is_closed_(false), + is_unregistered_(false), dependency_factory_(dependency_factory), track_adapter_map_( new WebRtcMediaStreamTrackAdapterMap(dependency_factory_, @@ -1055,6 +1056,12 @@ RTCPeerConnectionHandler::RTCPeerConnectionHandler( } RTCPeerConnectionHandler::~RTCPeerConnectionHandler() { + if (!is_unregistered_) { + StopAndUnregister(); + } +} + +void RTCPeerConnectionHandler::StopAndUnregister() { DCHECK(task_runner_->RunsTasksInCurrentSequence()); Stop(); @@ -1065,6 +1072,10 @@ RTCPeerConnectionHandler::~RTCPeerConnectionHandler() { UMA_HISTOGRAM_COUNTS_10000( "WebRTC.NumDataChannelsPerPeerConnection", num_data_channels_created_); + // Clear the pointer to client_ so that it does not interfere with + // garbage collection. + client_ = nullptr; + is_unregistered_ = true; } void RTCPeerConnectionHandler::associateWithFrame(blink::WebLocalFrame* frame) { @@ -1998,6 +2009,10 @@ void RTCPeerConnectionHandler::OnSignalingChange( peer_connection_tracker_->TrackSignalingStateChange(this, new_state); if (!is_closed_) client_->DidChangeSignalingState(new_state); + // The callback may have closed the PC. If so, do not continue. + if (is_closed_ || !client_) { + return; + } } // Called any time the IceConnectionState changes diff --git a/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.h b/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.h index 74cb1b77c1a..c6700cb154d 100644 --- a/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.h +++ b/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.h @@ -162,6 +162,8 @@ class CONTENT_EXPORT RTCPeerConnectionHandler const blink::WebString& label, const blink::WebRTCDataChannelInit& init) override; void Stop() override; + void StopAndUnregister() override; + blink::WebString Id() const override; // Delegate functions to allow for mocking of WebKit interfaces. @@ -302,14 +304,18 @@ class CONTENT_EXPORT RTCPeerConnectionHandler // first call fails. bool initialize_called_; - // |client_| is a weak pointer to the blink object (blink::RTCPeerConnection) + // |client_| is a raw pointer to the blink object (blink::RTCPeerConnection) // that owns this object. - // It is valid for the lifetime of this object. - blink::WebRTCPeerConnectionHandlerClient* const client_; + // It is valid for the lifetime of this object, but is cleared when + // StopAndUnregister() is called, in order to make sure it doesn't + // interfere with garbage collection of the owner object. + blink::WebRTCPeerConnectionHandlerClient* client_; // True if this PeerConnection has been closed. // After the PeerConnection has been closed, this object may no longer // forward callbacks to blink. bool is_closed_; + // True if StopAndUnregister has been called. + bool is_unregistered_; // |dependency_factory_| is a raw pointer, and is valid for the lifetime of // RenderThreadImpl. diff --git a/chromium/third_party/blink/public/platform/web_rtc_peer_connection_handler.h b/chromium/third_party/blink/public/platform/web_rtc_peer_connection_handler.h index 9c0e27aa3d7..c147fd3a39b 100644 --- a/chromium/third_party/blink/public/platform/web_rtc_peer_connection_handler.h +++ b/chromium/third_party/blink/public/platform/web_rtc_peer_connection_handler.h @@ -121,6 +121,12 @@ class WebRTCPeerConnectionHandler { virtual webrtc::RTCErrorOr<std::unique_ptr<WebRTCRtpTransceiver>> RemoveTrack( WebRTCRtpSender*) = 0; virtual void Stop() = 0; + // This function should be called when the object is taken out of service. + // There might be functions that need to return through the object, so it + // cannot be deleted yet, but no new operations should be allowed. + // All references to the object except the owning reference are deleted + // by this function. + virtual void StopAndUnregister() = 0; // Origin Trial - RtcPeerConnectionId virtual WebString Id() const = 0; 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 1506e6e6346..c1b374ed62d 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 @@ -629,9 +629,11 @@ RTCPeerConnection::~RTCPeerConnection() { } void RTCPeerConnection::Dispose() { - // Promptly clears a raw reference from content/ to an on-heap object + // Promptly clears the handler's pointer to |this| // so that content/ doesn't access it in a lazy sweeping phase. - peer_handler_.reset(); + if (peer_handler_) { + peer_handler_->StopAndUnregister(); + } } ScriptPromise RTCPeerConnection::createOffer(ScriptState* script_state, @@ -2201,7 +2203,7 @@ void RTCPeerConnection::ReleasePeerConnectionHandler() { dispatch_scheduled_event_runner_->Stop(); - peer_handler_.reset(); + peer_handler_->StopAndUnregister(); connection_handle_for_scheduler_.reset(); } |