Fix crash of ObjC SDK sLD / sRD with incorrect SDP.
There are two problems with setLocalDescription / setRemoteDescription
in ObjC SDK.
First, RTCSessionDescription.nativeDescription returns a raw
nullableSessionDescriptionInterface pointer, where sLD/sRD are calling
Clone() method unconditionally, so it might crash.
Second, unnecessary sLD/sRD calls Clone() of the raw pointer and
does not delete it, so this pointer will leak.
To solve these problems, I changed the return type of nativeDescription to
std::unique_ptr and removed the call to Clone() method.
Bug: webrtc:13022, webrtc:13035
Change-Id: Icbb87dda62d3a11af47ec74621cf64b8a6c05228
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227380
Reviewed-by: Kári Helgason <kthelgason@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Byoungchan Lee <daniel.l@hpcnt.com>
Cr-Commit-Position: refs/heads/master@{#34647}
diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection.mm b/sdk/objc/api/peerconnection/RTCPeerConnection.mm
index 05fe581..ee40135 100644
--- a/sdk/objc/api/peerconnection/RTCPeerConnection.mm
+++ b/sdk/objc/api/peerconnection/RTCPeerConnection.mm
@@ -581,7 +581,7 @@
RTC_DCHECK(completionHandler != nil);
rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface> observer(
new rtc::RefCountedObject<::SetSessionDescriptionObserver>(completionHandler));
- _peerConnection->SetLocalDescription(sdp.nativeDescription->Clone(), observer);
+ _peerConnection->SetLocalDescription(sdp.nativeDescription, observer);
}
- (void)setLocalDescriptionWithCompletionHandler:
@@ -597,7 +597,7 @@
RTC_DCHECK(completionHandler != nil);
rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface> observer(
new rtc::RefCountedObject<::SetSessionDescriptionObserver>(completionHandler));
- _peerConnection->SetRemoteDescription(sdp.nativeDescription->Clone(), observer);
+ _peerConnection->SetRemoteDescription(sdp.nativeDescription, observer);
}
- (BOOL)setBweMinBitrateBps:(nullable NSNumber *)minBitrateBps
diff --git a/sdk/objc/api/peerconnection/RTCSessionDescription+Private.h b/sdk/objc/api/peerconnection/RTCSessionDescription+Private.h
index 0f0a06a..aa087e5 100644
--- a/sdk/objc/api/peerconnection/RTCSessionDescription+Private.h
+++ b/sdk/objc/api/peerconnection/RTCSessionDescription+Private.h
@@ -22,7 +22,8 @@
* RTCSessionDescription object. This is needed to pass to the underlying C++
* APIs.
*/
- @property(nonatomic, readonly, nullable) webrtc::SessionDescriptionInterface *nativeDescription;
+ @property(nonatomic,
+ readonly) std::unique_ptr<webrtc::SessionDescriptionInterface> nativeDescription;
/**
* Initialize an RTCSessionDescription from a native
diff --git a/sdk/objc/api/peerconnection/RTCSessionDescription.mm b/sdk/objc/api/peerconnection/RTCSessionDescription.mm
index 9fd97fe..4ff02e8 100644
--- a/sdk/objc/api/peerconnection/RTCSessionDescription.mm
+++ b/sdk/objc/api/peerconnection/RTCSessionDescription.mm
@@ -46,13 +46,11 @@
#pragma mark - Private
-- (webrtc::SessionDescriptionInterface *)nativeDescription {
+- (std::unique_ptr<webrtc::SessionDescriptionInterface>)nativeDescription {
webrtc::SdpParseError error;
- webrtc::SessionDescriptionInterface *description =
- webrtc::CreateSessionDescription([[self class] stdStringForType:_type],
- _sdp.stdString,
- &error);
+ std::unique_ptr<webrtc::SessionDescriptionInterface> description(webrtc::CreateSessionDescription(
+ [[self class] stdStringForType:_type], _sdp.stdString, &error));
if (!description) {
RTCLogError(@"Failed to create session description: %s\nline: %s",
diff --git a/sdk/objc/unittests/RTCPeerConnectionTest.mm b/sdk/objc/unittests/RTCPeerConnectionTest.mm
index e45ca93..3452b96 100644
--- a/sdk/objc/unittests/RTCPeerConnectionTest.mm
+++ b/sdk/objc/unittests/RTCPeerConnectionTest.mm
@@ -23,11 +23,13 @@
#import "api/peerconnection/RTCPeerConnection.h"
#import "api/peerconnection/RTCPeerConnectionFactory+Native.h"
#import "api/peerconnection/RTCPeerConnectionFactory.h"
+#import "api/peerconnection/RTCSessionDescription.h"
#import "helpers/NSString+StdString.h"
@interface RTCPeerConnectionTest : NSObject
- (void)testConfigurationGetter;
- (void)testWithDependencies;
+- (void)testWithInvalidSDP;
@end
@implementation RTCPeerConnectionTest
@@ -137,6 +139,35 @@
}
}
+- (void)testWithInvalidSDP {
+ 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 setRemoteDescription:[[RTC_OBJC_TYPE(RTCSessionDescription) alloc]
+ initWithType:RTCSdpTypeOffer
+ sdp:@"invalid"]
+ 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) {
@@ -152,3 +183,10 @@
[test testWithDependencies];
}
}
+
+TEST(RTCPeerConnectionTest, TestWithInvalidSDP) {
+ @autoreleasepool {
+ RTCPeerConnectionTest *test = [[RTCPeerConnectionTest alloc] init];
+ [test testWithInvalidSDP];
+ }
+}
diff --git a/sdk/objc/unittests/RTCSessionDescriptionTest.mm b/sdk/objc/unittests/RTCSessionDescriptionTest.mm
index ee65649..25d7ffe 100644
--- a/sdk/objc/unittests/RTCSessionDescriptionTest.mm
+++ b/sdk/objc/unittests/RTCSessionDescriptionTest.mm
@@ -31,7 +31,7 @@
RTC_OBJC_TYPE(RTCSessionDescription) *description =
[[RTC_OBJC_TYPE(RTCSessionDescription) alloc] initWithType:RTCSdpTypeAnswer sdp:[self sdp]];
- webrtc::SessionDescriptionInterface *nativeDescription =
+ std::unique_ptr<webrtc::SessionDescriptionInterface> nativeDescription =
description.nativeDescription;
EXPECT_EQ(RTCSdpTypeAnswer,
