[Backport] Dependency for CVE-2020-6534 (2/4)

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/1570172:
Interpret getStats(callback, nonsense) as getStats(callback).

This fixes the regression described in the referenced bug.

The rtc_peer_connection.idl used to rely on
[LegacyInterfaceTypeChecking] which would make invalid second arguments,
such as the common misuse of an "errorCallback", be accepted by ignoring
the second argument. When this was removed, this caused regressions in
popular apps that incorrectly assumed the second argument was also a
callback.

In order to not break apps, this CL updates the custom bindings logic
to allow nonsense second arguments by ignoring them instead of throwing
TypeError. This continues not to inform misuses of the legacy API, but
making this API more type-friendly would require more use counters and
deprecation warnings.

Deprecation warnings can wait until we are ready to remove the legacy
API altogether.

Bug: 953512
Change-Id: Ie211cf8a4921812e5d243a513fbd95b224885440
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#651688}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

3 files changed, 51 insertions, 46 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 0014425eef7..0ff8ee649ed 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
@@ -1362,28 +1362,43 @@ bool RTCPeerConnection::IsRemoteStream(MediaStream* stream) const {
   return false;
 }
 
-ScriptPromise RTCPeerConnection::getStats(
-    ScriptState* script_state,
-    blink::ScriptValue callback_or_selector) {
-  auto argument = callback_or_selector.V8Value();
+ScriptPromise RTCPeerConnection::getStats(ScriptState* script_state) {
+  return getStats(
+      script_state,
+      ScriptValue(script_state, v8::Undefined(script_state->GetIsolate())),
+      ScriptValue(script_state, v8::Undefined(script_state->GetIsolate())));
+}
+
+ScriptPromise RTCPeerConnection::getStats(ScriptState* script_state,
+                                          ScriptValue callback_or_selector) {
+  return getStats(
+      script_state, std::move(callback_or_selector),
+      ScriptValue(script_state, v8::Undefined(script_state->GetIsolate())));
+}
+
+ScriptPromise RTCPeerConnection::getStats(ScriptState* script_state,
+                                          ScriptValue callback_or_selector,
+                                          ScriptValue legacy_selector) {
+  auto* isolate = script_state->GetIsolate();
+  auto first_argument = callback_or_selector.V8Value();
   // Custom binding for legacy "getStats(RTCStatsCallback callback)".
-  if (argument->IsFunction()) {
+  if (first_argument->IsFunction()) {
     V8RTCStatsCallback* success_callback =
-        V8RTCStatsCallback::Create(argument.As<v8::Function>());
-    return LegacyCallbackBasedGetStats(script_state, success_callback, nullptr);
-  }
-  // Custom binding for spec-compliant "getStats()" and "getStats(undefined)".
-  if (argument->IsUndefined())
-    return PromiseBasedGetStats(script_state, nullptr);
-  auto* isolate = callback_or_selector.GetIsolate();
-  // Custom binding for spec-compliant "getStats(MediaStreamTrack? selector)".
-  // null is a valid selector value, but value of wrong type isn't. |selector|
-  // set to no value means type error.
-  if (argument->IsNull())
+        V8RTCStatsCallback::Create(first_argument.As<v8::Function>());
+    MediaStreamTrack* selector_or_null =
+        V8MediaStreamTrack::ToImplWithTypeCheck(isolate,
+                                                legacy_selector.V8Value());
+    return LegacyCallbackBasedGetStats(script_state, success_callback,
+                                       selector_or_null);
+  }
+  // Custom binding for spec-compliant
+  // "getStats(optional MediaStreamTrack? selector)". null is a valid selector
+  // value, but a value of the wrong type isn't.
+  if (first_argument->IsNullOrUndefined())
     return PromiseBasedGetStats(script_state, nullptr);
 
   MediaStreamTrack* track =
-      V8MediaStreamTrack::ToImplWithTypeCheck(isolate, argument);
+      V8MediaStreamTrack::ToImplWithTypeCheck(isolate, first_argument);
   if (track)
     return PromiseBasedGetStats(script_state, track);
 
@@ -1395,17 +1410,6 @@ ScriptPromise RTCPeerConnection::getStats(
   return ScriptPromise::Reject(script_state, exception_state);
 }
 
-ScriptPromise RTCPeerConnection::getStats(ScriptState* script_state,
-                                          V8RTCStatsCallback* success_callback,
-                                          MediaStreamTrack* selector) {
-  return LegacyCallbackBasedGetStats(script_state, success_callback, selector);
-}
-
-ScriptPromise RTCPeerConnection::getStats(ScriptState* script_state,
-                                          MediaStreamTrack* selector) {
-  return PromiseBasedGetStats(script_state, selector);
-}
-
 ScriptPromise RTCPeerConnection::LegacyCallbackBasedGetStats(
     ScriptState* script_state,
     V8RTCStatsCallback* success_callback,
diff --git a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h
index 0c170bb7499..10d3b9ea623 100644
--- a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h
+++ b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h
@@ -162,15 +162,14 @@ class MODULES_EXPORT RTCPeerConnection final
 
   String id(ScriptState*) const;
 
-  // Calls one of the below versions (or rejects with an exception) depending on
-  // type, see RTCPeerConnection.idl.
-  ScriptPromise getStats(ScriptState*, blink::ScriptValue callback_or_selector);
-  // Calls LegacyCallbackBasedGetStats().
-  ScriptPromise getStats(ScriptState*,
-                         V8RTCStatsCallback* success_callback,
-                         MediaStreamTrack* selector = nullptr);
-  // Calls PromiseBasedGetStats().
-  ScriptPromise getStats(ScriptState*, MediaStreamTrack* selector = nullptr);
+  // Calls LegacyCallbackBasedGetStats() or PromiseBasedGetStats() (or rejects
+  // with an exception) depending on type, see rtc_peer_connection.idl.
+  ScriptPromise getStats(ScriptState* script_state);
+  ScriptPromise getStats(ScriptState* script_state,
+                         ScriptValue callback_or_selector);
+  ScriptPromise getStats(ScriptState* script_state,
+                         ScriptValue callback_or_selector,
+                         ScriptValue legacy_selector);
   ScriptPromise LegacyCallbackBasedGetStats(
       ScriptState*,
       V8RTCStatsCallback* success_callback,
diff --git a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.idl b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.idl
index 35913caf0d8..cbf22c20cfd 100644
--- a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.idl
+++ b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.idl
@@ -104,23 +104,25 @@ enum RTCIceConnectionState {
     [CallWith=ScriptState] Promise<void> setRemoteDescription(RTCSessionDescriptionInit description, VoidFunction successCallback, optional RTCPeerConnectionErrorCallback? failureCallback);
     [CallWith=ScriptState, RaisesException, MeasureAs=RTCPeerConnectionAddIceCandidateLegacy] Promise<void> addIceCandidate((RTCIceCandidateInit or RTCIceCandidate) candidate, VoidFunction successCallback, RTCPeerConnectionErrorCallback failureCallback);
 
-    // Legacy getStats() API. The returned metrics are a completely different
-    // set of metrics than the standard compliant version, presented in a
-    // different format. They are undocumented, implementation-specific and
+    // getStats() has a standardized version and a legacy non-standard version.
+    //
+    // In the legacy getStats() API the returned metrics are a completely
+    // different set of metrics than the standard compliant version, presented
+    // in a different format. They are undocumented, implementation-specific and
     // should go away but it is still heavily used. The selector argument can
     // optionally be used to filter the results only to return metrics relevant
     // for the selector.
     // TODO(hbos): Deprecate and remove this API. https://crbug.com/822696
-    [CallWith=ScriptState, LegacyInterfaceTypeChecking] Promise<void> getStats(RTCStatsCallback successCallback, MediaStreamTrack? selector);
+    //
     // Due to a limitation of generated V8 bindings (https://crbug.com/828401),
     // it is not possible to express both legacy and spec-compliant versions of
     // getStats() in IDL. This version implements two different APIs with custom
-    // bindings to resolve which one to call in RTCPeerConnection.cpp:
+    // bindings to resolve which one to call in rtc_peer_connection.cc:
     //
     // 1. Promise<void> getStats(RTCStatsCallback successCallback, optional MediaStreamTrack? selector);
-    //    This is the legacy getStats() API handling the case when the selector
-    //    argument is missing, for more details on the legacy API and the IDL
-    //    for when the selector is present, see above.
+    //    Promise<void> getStats(RTCStatsCallback successCallback, any ignoredArgument)
+    //    This is the legacy getStats() API, for more details on the legacy API
+    //    and the IDL for when the selector is present, see above.
     //
     // 2. Promise<RTCStatsReport> getStats(optional MediaStreamTrack? selector = null);
     //    This is the spec-compliant version of getStats(). Spec for API:
@@ -129,7 +131,7 @@ enum RTCIceConnectionState {
     //    List of implemented stats:
     //    https://cs.chromium.org/chromium/src/third_party/webrtc/api/stats/rtcstats_objects.h
     //    See also RTCRtpSender.getStats() and RTCRtpReceiver.getStats().
-    [CallWith=ScriptState] Promise<any> getStats(optional any callbackOrSelector);
+    [CallWith=ScriptState] Promise<any> getStats(optional any callbackOrSelector, optional any legacySelector);
 
     // RTP Media API
     // https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-gettransceivers