Darwin: Keep MTRCommissionableBrowser around until OnBleScanStopped (#…

…33674)

* 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.

* restyle

* Retain owner in Stop() instead of Start()

* More review comments

src/darwin/Framework/CHIP/MTRCommissionableBrowser.mm

@@ -61,6 +61,10 @@ @implementation MTRCommissionableBrowserResult
 #endif // CONFIG_NETWORK_LAYER_BLE
 {
 public:
+#if CONFIG_NETWORK_LAYER_BLE
+    id mBleScannerDelegateOwner;
+#endif // CONFIG_NETWORK_LAYER_BLE
+
     CHIP_ERROR Start(id<MTRCommissionableBrowserDelegate> delegate, MTRDeviceController * controller, dispatch_queue_t queue)
     {
         assertChipStackLockedByCurrentThread();
@@ -90,7 +94,7 @@ CHIP_ERROR Start(id<MTRCommissionableBrowserDelegate> delegate, MTRDeviceControl
             chip::Inet::InterfaceId::Null(), this);
     }
 
-    CHIP_ERROR Stop()
+    CHIP_ERROR Stop(id owner)
     {
         assertChipStackLockedByCurrentThread();
 
@@ -109,7 +113,10 @@ CHIP_ERROR Stop()
         mDiscoveredResults = nil;
 
 #if CONFIG_NETWORK_LAYER_BLE
-        ReturnErrorOnFailure(PlatformMgrImpl().StopBleScan());
+        mBleScannerDelegateOwner = owner; // retain the owner until OnBleScanStopped is called
+        PlatformMgrImpl().StopBleScan(); // doesn't actually fail, and if it did we'd want to carry on regardless
+#else
+        (void) owner;
 #endif // CONFIG_NETWORK_LAYER_BLE
 
         return ChipDnssdStopBrowse(this);
@@ -281,6 +288,7 @@ void OnBrowseStop(CHIP_ERROR error) override
     void OnBleScanAdd(BLE_CONNECTION_OBJECT connObj, const ChipBLEDeviceIdentificationInfo & info) override
     {
         assertChipStackLockedByCurrentThread();
+        VerifyOrReturn(mDelegate != nil);
 
         auto result = [[MTRCommissionableBrowserResult alloc] init];
         result.instanceName = [NSString stringWithUTF8String:kBleKey];
@@ -303,6 +311,7 @@ void OnBleScanAdd(BLE_CONNECTION_OBJECT connObj, const ChipBLEDeviceIdentificati
     void OnBleScanRemove(BLE_CONNECTION_OBJECT connObj) override
     {
         assertChipStackLockedByCurrentThread();
+        VerifyOrReturn(mDelegate != nil);
 
         auto key = [NSString stringWithFormat:@"%@", connObj];
         if ([mDiscoveredResults objectForKey:key] == nil) {
@@ -319,6 +328,12 @@ void OnBleScanRemove(BLE_CONNECTION_OBJECT connObj) override
             [mDelegate controller:mController didFindCommissionableDevice:result];
         });
     }
+
+    void OnBleScanStopped() override
+    {
+        mBleScannerDelegateOwner = nil;
+    }
+
 #endif // CONFIG_NETWORK_LAYER_BLE
 
 private:
@@ -360,7 +375,7 @@ - (BOOL)start
 
 - (BOOL)stop
 {
-    VerifyOrReturnValue(CHIP_NO_ERROR == _browser.Stop(), NO);
+    VerifyOrReturnValue(CHIP_NO_ERROR == _browser.Stop(self), NO);
     _delegate = nil;
     _controller = nil;
     _queue = nil;