From 8c487578f03687c93c88e1b21241365fcf5c6bf2 Mon Sep 17 00:00:00 2001
From: Byoungchan Lee <daniel.l@hpcnt.com>
Date: Wed, 4 Aug 2021 20:55:25 +0900
Subject: [PATCH] Fix crash of Objc SDK addIceCandidate with incorrect
 candidate.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RTCIceCandidate.nativeCandidate returns a unique_ptr that
can be null. As with the previous CL, this is used without checking
whether it is null or not, so it should be fixed.

Bug: None
Change-Id: I70a84f7a2a9a9d47b8cefa198204f9b6d63a7c7f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227620
Commit-Queue: Byoungchan Lee <daniel.l@hpcnt.com>
Reviewed-by: Kári Helgason <kthelgason@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34649}
---
 .../api/peerconnection/RTCPeerConnection.mm   | 26 ++++++------
 sdk/objc/unittests/RTCPeerConnectionTest.mm   | 40 +++++++++++++++++++
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection.mm b/sdk/objc/api/peerconnection/RTCPeerConnection.mm
index ee40135ff0..67d0ff0cd6 100644
--- a/sdk/objc/api/peerconnection/RTCPeerConnection.mm
+++ b/sdk/objc/api/peerconnection/RTCPeerConnection.mm
@@ -437,20 +437,18 @@ void PeerConnectionDelegateAdapter::OnRemoveTrack(
 - (void)addIceCandidate:(RTC_OBJC_TYPE(RTCIceCandidate) *)candidate
       completionHandler:(void (^)(NSError *_Nullable error))completionHandler {
   RTC_DCHECK(completionHandler != nil);
-  auto iceCandidate = webrtc::CreateIceCandidate(candidate.nativeCandidate->sdp_mid(),
-                                                 candidate.nativeCandidate->sdp_mline_index(),
-                                                 candidate.nativeCandidate->candidate());
-  _peerConnection->AddIceCandidate(std::move(iceCandidate), [completionHandler](const auto &error) {
-    if (error.ok()) {
-      completionHandler(nil);
-    } else {
-      NSString *str = [NSString stringForStdString:error.message()];
-      NSError *err = [NSError errorWithDomain:kRTCPeerConnectionErrorDomain
-                                         code:static_cast<NSInteger>(error.type())
-                                     userInfo:@{NSLocalizedDescriptionKey : str}];
-      completionHandler(err);
-    }
-  });
+  _peerConnection->AddIceCandidate(
+      candidate.nativeCandidate, [completionHandler](const auto &error) {
+        if (error.ok()) {
+          completionHandler(nil);
+        } else {
+          NSString *str = [NSString stringForStdString:error.message()];
+          NSError *err = [NSError errorWithDomain:kRTCPeerConnectionErrorDomain
+                                             code:static_cast<NSInteger>(error.type())
+                                         userInfo:@{NSLocalizedDescriptionKey : str}];
+          completionHandler(err);
+        }
+      });
 }
 - (void)removeIceCandidates:(NSArray<RTC_OBJC_TYPE(RTCIceCandidate) *> *)iceCandidates {
   std::vector<cricket::Candidate> candidates;
diff --git a/sdk/objc/unittests/RTCPeerConnectionTest.mm b/sdk/objc/unittests/RTCPeerConnectionTest.mm
index 3452b964d9..1d0ae2679e 100644
--- a/sdk/objc/unittests/RTCPeerConnectionTest.mm
+++ b/sdk/objc/unittests/RTCPeerConnectionTest.mm
@@ -18,6 +18,7 @@
 #import "api/peerconnection/RTCConfiguration+Private.h"
 #import "api/peerconnection/RTCConfiguration.h"
 #import "api/peerconnection/RTCCryptoOptions.h"
+#import "api/peerconnection/RTCIceCandidate.h"
 #import "api/peerconnection/RTCIceServer.h"
 #import "api/peerconnection/RTCMediaConstraints.h"
 #import "api/peerconnection/RTCPeerConnection.h"
@@ -30,6 +31,7 @@
 - (void)testConfigurationGetter;
 - (void)testWithDependencies;
 - (void)testWithInvalidSDP;
+- (void)testWithInvalidIceCandidate;
 @end
 
 @implementation RTCPeerConnectionTest
@@ -168,6 +170,37 @@
                               dispatch_time(DISPATCH_TIME_NOW, (int64_t)(timeout * NSEC_PER_SEC))));
   [peerConnection close];
 }
+
+- (void)testWithInvalidIceCandidate {
+  RTC_OBJC_TYPE(RTCPeerConnectionFactory) *factory =
+      [[RTC_OBJC_TYPE(RTCPeerConnectionFactory) alloc] init];
+
+  RTC_OBJC_TYPE(RTCConfiguration) *config = [[RTC_OBJC_TYPE(RTCConfiguration) alloc] init];
+  RTC_OBJC_TYPE(RTCMediaConstraints) *contraints =
+      [[RTC_OBJC_TYPE(RTCMediaConstraints) alloc] initWithMandatoryConstraints:@{}
+                                                           optionalConstraints:nil];
+  RTC_OBJC_TYPE(RTCPeerConnection) *peerConnection =
+      [factory peerConnectionWithConfiguration:config constraints:contraints delegate:nil];
+
+  dispatch_semaphore_t negotiatedSem = dispatch_semaphore_create(0);
+  [peerConnection addIceCandidate:[[RTC_OBJC_TYPE(RTCIceCandidate) alloc] initWithSdp:@"invalid"
+                                                                        sdpMLineIndex:-1
+                                                                               sdpMid:nil]
+                completionHandler:^(NSError *error) {
+                  ASSERT_NE(error, nil);
+                  if (error != nil) {
+                    dispatch_semaphore_signal(negotiatedSem);
+                  }
+                }];
+
+  NSTimeInterval timeout = 5;
+  ASSERT_EQ(
+      0,
+      dispatch_semaphore_wait(negotiatedSem,
+                              dispatch_time(DISPATCH_TIME_NOW, (int64_t)(timeout * NSEC_PER_SEC))));
+  [peerConnection close];
+}
+
 @end
 
 TEST(RTCPeerConnectionTest, ConfigurationGetterTest) {
@@ -190,3 +223,10 @@ TEST(RTCPeerConnectionTest, TestWithInvalidSDP) {
     [test testWithInvalidSDP];
   }
 }
+
+TEST(RTCPeerConnectionTest, TestWithInvalidIceCandidate) {
+  @autoreleasepool {
+    RTCPeerConnectionTest *test = [[RTCPeerConnectionTest alloc] init];
+    [test testWithInvalidIceCandidate];
+  }
+}