Skip to content

Commit 3d08de3

Browse files
Darwin: Keep MTRCommissionableBrowser around until OnBleScanStopped
Because the BLE platform implementation uses a separate dispatch queue, StopBleScan() does not synchrnously stop any further use of the current BleScannerDelegate. Keep the MTRCommissionableBrowser that contains the delegate alive until OnBleScanStopped to avoid a UAF.
1 parent 1f1b750 commit 3d08de3

File tree

1 file changed

+17
-2
lines changed

1 file changed

+17
-2
lines changed

src/darwin/Framework/CHIP/MTRCommissionableBrowser.mm

+17-2
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ @implementation MTRCommissionableBrowserResult
6161
#endif // CONFIG_NETWORK_LAYER_BLE
6262
{
6363
public:
64-
CHIP_ERROR Start(id<MTRCommissionableBrowserDelegate> delegate, MTRDeviceController * controller, dispatch_queue_t queue)
64+
#if CONFIG_NETWORK_LAYER_BLE
65+
id mBleScannerDelegateOwner;
66+
#endif // CONFIG_NETWORK_LAYER_BLE
67+
68+
CHIP_ERROR Start(id owner, id<MTRCommissionableBrowserDelegate> delegate, MTRDeviceController * controller, dispatch_queue_t queue)
6569
{
6670
assertChipStackLockedByCurrentThread();
6771

@@ -77,7 +81,10 @@ CHIP_ERROR Start(id<MTRCommissionableBrowserDelegate> delegate, MTRDeviceControl
7781
ResetCounters();
7882

7983
#if CONFIG_NETWORK_LAYER_BLE
84+
mBleScannerDelegateOwner = owner; // retain the owner until OnBleScanStopped is called (see rdar://128723029)
8085
ReturnErrorOnFailure(PlatformMgrImpl().StartBleScan(this));
86+
#else
87+
(void)owner;
8188
#endif // CONFIG_NETWORK_LAYER_BLE
8289

8390
ReturnErrorOnFailure(Resolver::Instance().Init(chip::DeviceLayer::UDPEndPointManager()));
@@ -281,6 +288,7 @@ void OnBrowseStop(CHIP_ERROR error) override
281288
void OnBleScanAdd(BLE_CONNECTION_OBJECT connObj, const ChipBLEDeviceIdentificationInfo & info) override
282289
{
283290
assertChipStackLockedByCurrentThread();
291+
VerifyOrReturn(mDelegate != nil);
284292

285293
auto result = [[MTRCommissionableBrowserResult alloc] init];
286294
result.instanceName = [NSString stringWithUTF8String:kBleKey];
@@ -303,6 +311,7 @@ void OnBleScanAdd(BLE_CONNECTION_OBJECT connObj, const ChipBLEDeviceIdentificati
303311
void OnBleScanRemove(BLE_CONNECTION_OBJECT connObj) override
304312
{
305313
assertChipStackLockedByCurrentThread();
314+
VerifyOrReturn(mDelegate != nil);
306315

307316
auto key = [NSString stringWithFormat:@"%@", connObj];
308317
if ([mDiscoveredResults objectForKey:key] == nil) {
@@ -319,6 +328,12 @@ void OnBleScanRemove(BLE_CONNECTION_OBJECT connObj) override
319328
[mDelegate controller:mController didFindCommissionableDevice:result];
320329
});
321330
}
331+
332+
void OnBleScanStopped() override
333+
{
334+
mBleScannerDelegateOwner = nil;
335+
}
336+
322337
#endif // CONFIG_NETWORK_LAYER_BLE
323338

324339
private:
@@ -354,7 +369,7 @@ - (instancetype)initWithDelegate:(id<MTRCommissionableBrowserDelegate>)delegate
354369

355370
- (BOOL)start
356371
{
357-
VerifyOrReturnValue(CHIP_NO_ERROR == _browser.Start(_delegate, _controller, _queue), NO);
372+
VerifyOrReturnValue(CHIP_NO_ERROR == _browser.Start(self, _delegate, _controller, _queue), NO);
358373
return YES;
359374
}
360375

0 commit comments

Comments
 (0)