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}
This commit is contained in:
Byoungchan Lee 2021-08-04 20:54:45 +09:00 committed by WebRTC LUCI CQ
parent 31c0cfdf7f
commit 33728152ca
5 changed files with 46 additions and 9 deletions

View file

@ -581,7 +581,7 @@ void PeerConnectionDelegateAdapter::OnRemoveTrack(
RTC_DCHECK(completionHandler != nil); RTC_DCHECK(completionHandler != nil);
rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface> observer( rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface> observer(
new rtc::RefCountedObject<::SetSessionDescriptionObserver>(completionHandler)); new rtc::RefCountedObject<::SetSessionDescriptionObserver>(completionHandler));
_peerConnection->SetLocalDescription(sdp.nativeDescription->Clone(), observer); _peerConnection->SetLocalDescription(sdp.nativeDescription, observer);
} }
- (void)setLocalDescriptionWithCompletionHandler: - (void)setLocalDescriptionWithCompletionHandler:
@ -597,7 +597,7 @@ void PeerConnectionDelegateAdapter::OnRemoveTrack(
RTC_DCHECK(completionHandler != nil); RTC_DCHECK(completionHandler != nil);
rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface> observer( rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface> observer(
new rtc::RefCountedObject<::SetSessionDescriptionObserver>(completionHandler)); new rtc::RefCountedObject<::SetSessionDescriptionObserver>(completionHandler));
_peerConnection->SetRemoteDescription(sdp.nativeDescription->Clone(), observer); _peerConnection->SetRemoteDescription(sdp.nativeDescription, observer);
} }
- (BOOL)setBweMinBitrateBps:(nullable NSNumber *)minBitrateBps - (BOOL)setBweMinBitrateBps:(nullable NSNumber *)minBitrateBps

View file

@ -22,7 +22,8 @@ NS_ASSUME_NONNULL_BEGIN
* RTCSessionDescription object. This is needed to pass to the underlying C++ * RTCSessionDescription object. This is needed to pass to the underlying C++
* APIs. * APIs.
*/ */
@property(nonatomic, readonly, nullable) webrtc::SessionDescriptionInterface *nativeDescription; @property(nonatomic,
readonly) std::unique_ptr<webrtc::SessionDescriptionInterface> nativeDescription;
/** /**
* Initialize an RTCSessionDescription from a native * Initialize an RTCSessionDescription from a native

View file

@ -46,13 +46,11 @@
#pragma mark - Private #pragma mark - Private
- (webrtc::SessionDescriptionInterface *)nativeDescription { - (std::unique_ptr<webrtc::SessionDescriptionInterface>)nativeDescription {
webrtc::SdpParseError error; webrtc::SdpParseError error;
webrtc::SessionDescriptionInterface *description = std::unique_ptr<webrtc::SessionDescriptionInterface> description(webrtc::CreateSessionDescription(
webrtc::CreateSessionDescription([[self class] stdStringForType:_type], [[self class] stdStringForType:_type], _sdp.stdString, &error));
_sdp.stdString,
&error);
if (!description) { if (!description) {
RTCLogError(@"Failed to create session description: %s\nline: %s", RTCLogError(@"Failed to create session description: %s\nline: %s",

View file

@ -23,11 +23,13 @@
#import "api/peerconnection/RTCPeerConnection.h" #import "api/peerconnection/RTCPeerConnection.h"
#import "api/peerconnection/RTCPeerConnectionFactory+Native.h" #import "api/peerconnection/RTCPeerConnectionFactory+Native.h"
#import "api/peerconnection/RTCPeerConnectionFactory.h" #import "api/peerconnection/RTCPeerConnectionFactory.h"
#import "api/peerconnection/RTCSessionDescription.h"
#import "helpers/NSString+StdString.h" #import "helpers/NSString+StdString.h"
@interface RTCPeerConnectionTest : NSObject @interface RTCPeerConnectionTest : NSObject
- (void)testConfigurationGetter; - (void)testConfigurationGetter;
- (void)testWithDependencies; - (void)testWithDependencies;
- (void)testWithInvalidSDP;
@end @end
@implementation RTCPeerConnectionTest @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 @end
TEST(RTCPeerConnectionTest, ConfigurationGetterTest) { TEST(RTCPeerConnectionTest, ConfigurationGetterTest) {
@ -152,3 +183,10 @@ TEST(RTCPeerConnectionTest, TestWithDependencies) {
[test testWithDependencies]; [test testWithDependencies];
} }
} }
TEST(RTCPeerConnectionTest, TestWithInvalidSDP) {
@autoreleasepool {
RTCPeerConnectionTest *test = [[RTCPeerConnectionTest alloc] init];
[test testWithInvalidSDP];
}
}

View file

@ -31,7 +31,7 @@
RTC_OBJC_TYPE(RTCSessionDescription) *description = RTC_OBJC_TYPE(RTCSessionDescription) *description =
[[RTC_OBJC_TYPE(RTCSessionDescription) alloc] initWithType:RTCSdpTypeAnswer sdp:[self sdp]]; [[RTC_OBJC_TYPE(RTCSessionDescription) alloc] initWithType:RTCSdpTypeAnswer sdp:[self sdp]];
webrtc::SessionDescriptionInterface *nativeDescription = std::unique_ptr<webrtc::SessionDescriptionInterface> nativeDescription =
description.nativeDescription; description.nativeDescription;
EXPECT_EQ(RTCSdpTypeAnswer, EXPECT_EQ(RTCSdpTypeAnswer,