[Backport] CVE-2019-13694

Fix heap-use-after-free in setLocalDescription/setRemoteDescription.

This is another case where the pc handler invokes JavaScript callbacks
which could cause the PC+handler to be deleted. The fix is to invoke the
callback as the last step before returning.

(cherry picked from commit 0cd560eea3e00305765c2a9da7ec959ccb757460)

Bug: 1005251
Change-Id: I9a06ed0a6885b2f6d46e6646c2df0a9d07e79a2d
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#701778}
Cr-Commit-Position: refs/branch-heads/3865@{#869}
Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094}
Reviewed-by: Kirill Burtsev <kirill.burtsev@qt.io>

1 files changed, 12 insertions, 4 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 7695d52bc13..5647813ffb6 100644
--- a/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
+++ b/chromium/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
@@ -1199,13 +1199,17 @@ void RTCPeerConnectionHandler::SetLocalDescription(
     reason_str.append(" ");
     reason_str.append(error.description);
     LOG(ERROR) << reason_str;
-    request.RequestFailed(webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR,
-                                           std::move(reason_str)));
     if (peer_connection_tracker_) {
       peer_connection_tracker_->TrackSessionDescriptionCallback(
           this, PeerConnectionTracker::ACTION_SET_LOCAL_DESCRIPTION,
           "OnFailure", reason_str);
     }
+    // Warning: this line triggers the error callback to be executed, causing
+    // arbitrary JavaScript to be executed synchronously. As a result, it is
+    // possible for |this| to be deleted after this line. See
+    // https://crbug.com/1005251.
+    request.RequestFailed(webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR,
+                                           std::move(reason_str)));
     return;
   }
 
@@ -1267,13 +1271,17 @@ void RTCPeerConnectionHandler::SetRemoteDescription(
     reason_str.append(" ");
     reason_str.append(error.description);
     LOG(ERROR) << reason_str;
-    request.RequestFailed(webrtc::RTCError(
-        webrtc::RTCErrorType::UNSUPPORTED_OPERATION, std::move(reason_str)));
     if (peer_connection_tracker_) {
       peer_connection_tracker_->TrackSessionDescriptionCallback(
           this, PeerConnectionTracker::ACTION_SET_REMOTE_DESCRIPTION,
           "OnFailure", reason_str);
     }
+    // Warning: this line triggers the error callback to be executed, causing
+    // arbitrary JavaScript to be executed synchronously. As a result, it is
+    // possible for |this| to be deleted after this line. See
+    // https://crbug.com/1005251.
+    request.RequestFailed(webrtc::RTCError(
+        webrtc::RTCErrorType::UNSUPPORTED_OPERATION, std::move(reason_str)));
     return;
   }