Skip to content

Commit 0b28ae4

Browse files
authored
Merge branch 'master' into master
2 parents 8fd2d65 + c0e580c commit 0b28ae4

24 files changed

+1430
-551
lines changed

.github/workflows/darwin.yaml

+1-2
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,10 @@ jobs:
116116
117117
export TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1
118118
119-
# Disable BLE (CHIP_IS_BLE=NO) because the app does not have the permission to use it and that may crash the CI.
120119
xcodebuild test -target "Matter" -scheme "Matter Framework Tests" \
121120
-resultBundlePath /tmp/darwin/framework-tests/TestResults.xcresult \
122121
-sdk macosx ${{ matrix.options.arguments }} \
123-
CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} ${{ matrix.options.defines }}' \
122+
GCC_PREPROCESSOR_DEFINITIONS='${inherited} ${{ matrix.options.defines }}' \
124123
> >(tee /tmp/darwin/framework-tests/darwin-tests.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-err.log >&2)
125124
- name: Generate Summary
126125
if: always()

src/controller/python/chip/ble/darwin/Scanning.mm

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#include <ble/Ble.h>
22
#include <lib/support/CHIPMem.h>
33
#include <lib/support/logging/CHIPLogging.h>
4-
#include <platform/Darwin/MTRUUIDHelper.h>
4+
#include <platform/Darwin/BleUtils.h>
55

66
#import <CoreBluetooth/CoreBluetooth.h>
77

@@ -45,7 +45,7 @@ - (id)initWithContext:(PyObject *)context
4545
{
4646
self = [super init];
4747
if (self) {
48-
self.shortServiceUUID = [MTRUUIDHelper GetShortestServiceUUID:&chip::Ble::CHIP_BLE_SVC_ID];
48+
self.shortServiceUUID = chip::DeviceLayer::Internal::CBUUIDFromBleUUID(chip::Ble::CHIP_BLE_SVC_ID);
4949

5050
_workQueue = dispatch_queue_create("com.chip.python.ble.work_queue", DISPATCH_QUEUE_SERIAL);
5151
_timer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, _workQueue);
@@ -78,7 +78,7 @@ - (void)centralManager:(CBCentralManager *)central
7878

7979
NSDictionary * servicesData = [advertisementData objectForKey:CBAdvertisementDataServiceDataKey];
8080
for (CBUUID * serviceUUID in servicesData) {
81-
if (![serviceUUID.data isEqualToData:_shortServiceUUID.data]) {
81+
if (![serviceUUID isEqualTo:_shortServiceUUID]) {
8282
continue;
8383
}
8484
NSData * serviceData = [servicesData objectForKey:serviceUUID];

src/darwin/Framework/CHIP/MTRCommissionableBrowser.mm

+6
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@
2626
#include <controller/CHIPDeviceController.h>
2727
#include <lib/dnssd/platform/Dnssd.h>
2828
#include <platform/CHIPDeviceLayer.h>
29+
#include <platform/Darwin/BleUtils.h>
2930

3031
using namespace chip::Dnssd;
3132
using namespace chip::DeviceLayer;
33+
using namespace chip::DeviceLayer::Internal;
3234

3335
#if CONFIG_NETWORK_LAYER_BLE
3436
#include <platform/Darwin/BleScannerDelegate.h>
@@ -39,6 +41,8 @@
3941

4042
using namespace chip::Tracing::DarwinFramework;
4143

44+
@class CBPeripheral;
45+
4246
@implementation MTRCommissionableBrowserResultInterfaces
4347
@end
4448

@@ -48,6 +52,7 @@ @interface MTRCommissionableBrowserResult ()
4852
@property (nonatomic) NSNumber * productID;
4953
@property (nonatomic) NSNumber * discriminator;
5054
@property (nonatomic) BOOL commissioningMode;
55+
@property (nonatomic, strong, nullable) CBPeripheral * peripheral;
5156
@end
5257

5358
@implementation MTRCommissionableBrowserResult
@@ -302,6 +307,7 @@ void OnBleScanAdd(BLE_CONNECTION_OBJECT connObj, const ChipBLEDeviceIdentificati
302307
result.discriminator = @(info.GetDeviceDiscriminator());
303308
result.commissioningMode = YES;
304309
result.params = chip::MakeOptional(chip::Controller::SetUpCodePairerParameters(connObj, false /* connected */));
310+
result.peripheral = CBPeripheralFromBleConnObject(connObj); // avoid params holding a dangling pointer
305311

306312
MATTER_LOG_METRIC(kMetricBLEDevicesAdded, ++mBLEDevicesAdded);
307313

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
/**
2+
* Copyright (c) 2024 Project CHIP Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#import "MTRMockCB.h"
18+
#import "MTRTestCase.h"
19+
#import "MTRTestKeys.h"
20+
#import "MTRTestStorage.h"
21+
22+
#import <Matter/Matter.h>
23+
#import <XCTest/XCTest.h>
24+
#import <stdatomic.h>
25+
26+
NS_ASSUME_NONNULL_BEGIN
27+
28+
@interface MTRBleTests : MTRTestCase
29+
@end
30+
31+
@interface TestBrowserDelegate : NSObject <MTRCommissionableBrowserDelegate>
32+
@property (strong) void (^onDidFindCommissionableDevice)(MTRDeviceController *, MTRCommissionableBrowserResult *);
33+
@property (strong) void (^onDidRemoveCommissionableDevice)(MTRDeviceController *, MTRCommissionableBrowserResult *);
34+
@end
35+
36+
@implementation TestBrowserDelegate
37+
38+
- (void)controller:(nonnull MTRDeviceController *)controller didFindCommissionableDevice:(MTRCommissionableBrowserResult *)device
39+
{
40+
__auto_type block = self.onDidFindCommissionableDevice;
41+
if (block) {
42+
block(controller, device);
43+
}
44+
}
45+
46+
- (void)controller:(nonnull MTRDeviceController *)controller didRemoveCommissionableDevice:(MTRCommissionableBrowserResult *)device
47+
{
48+
__auto_type block = self.onDidRemoveCommissionableDevice;
49+
if (block) {
50+
block(controller, device);
51+
}
52+
}
53+
54+
@end
55+
56+
MTRDeviceController * sController;
57+
58+
@implementation MTRBleTests
59+
60+
- (void)setUp
61+
{
62+
[super setUp];
63+
64+
[self.class.mockCoreBluetooth reset];
65+
66+
sController = [MTRTestCase createControllerOnTestFabric];
67+
XCTAssertNotNil(sController);
68+
}
69+
70+
- (void)tearDown
71+
{
72+
[sController shutdown];
73+
sController = nil;
74+
[[MTRDeviceControllerFactory sharedInstance] stopControllerFactory];
75+
76+
[super tearDown];
77+
}
78+
79+
- (void)testBleCommissionableBrowserResultAdditionAndRemoval
80+
{
81+
__block MTRCommissionableBrowserResult * device;
82+
XCTestExpectation * didFindDevice = [self expectationWithDescription:@"did find device"];
83+
TestBrowserDelegate * delegate = [[TestBrowserDelegate alloc] init];
84+
delegate.onDidFindCommissionableDevice = ^(MTRDeviceController * controller, MTRCommissionableBrowserResult * result) {
85+
if ([result.instanceName isEqualToString:@"BLE"]) { // TODO: This is a lame API
86+
XCTAssertNil(device);
87+
XCTAssertEqualObjects(result.vendorID, @0xfff1);
88+
XCTAssertEqualObjects(result.productID, @0x1234);
89+
XCTAssertEqualObjects(result.discriminator, @0x444);
90+
device = result;
91+
[didFindDevice fulfill];
92+
}
93+
};
94+
95+
XCTestExpectation * didRemoveDevice = [self expectationWithDescription:@"did remove device"];
96+
delegate.onDidRemoveCommissionableDevice = ^(MTRDeviceController * controller, MTRCommissionableBrowserResult * result) {
97+
if ([result.instanceName isEqualToString:@"BLE"]) {
98+
XCTAssertNotNil(device);
99+
XCTAssertEqual(result, device);
100+
[didRemoveDevice fulfill];
101+
}
102+
};
103+
104+
XCTAssertTrue([sController startBrowseForCommissionables:delegate queue:dispatch_get_main_queue()]);
105+
106+
NSUUID * peripheralID = [NSUUID UUID];
107+
[self.class.mockCoreBluetooth addMockCommissionableMatterDeviceWithIdentifier:peripheralID vendorID:@0xfff1 productID:@0x1234 discriminator:@0x444];
108+
[self.class.mockCoreBluetooth removeMockPeripheralWithIdentifier:peripheralID];
109+
110+
// BleConnectionDelegateImpl kCachePeripheralTimeoutInSeconds is approximately 10 seconds
111+
[self waitForExpectations:@[ didFindDevice, didRemoveDevice ] timeout:14 enforceOrder:YES];
112+
XCTAssertTrue([sController stopBrowseForCommissionables]);
113+
}
114+
115+
- (void)testBleCommissionAfterStopBrowseUAF
116+
{
117+
__block MTRCommissionableBrowserResult * device;
118+
XCTestExpectation * didFindDevice = [self expectationWithDescription:@"did find device"];
119+
TestBrowserDelegate * delegate = [[TestBrowserDelegate alloc] init];
120+
delegate.onDidFindCommissionableDevice = ^(MTRDeviceController * controller, MTRCommissionableBrowserResult * result) {
121+
if ([result.instanceName isEqualToString:@"BLE"]) {
122+
XCTAssertNil(device);
123+
XCTAssertEqualObjects(result.vendorID, @0xfff1);
124+
XCTAssertEqualObjects(result.productID, @0x1234);
125+
XCTAssertEqualObjects(result.discriminator, @0x444);
126+
device = result;
127+
[didFindDevice fulfill];
128+
}
129+
};
130+
131+
XCTAssertTrue([sController startBrowseForCommissionables:delegate queue:dispatch_get_main_queue()]);
132+
133+
NSUUID * peripheralID = [NSUUID UUID];
134+
[self.class.mockCoreBluetooth addMockCommissionableMatterDeviceWithIdentifier:peripheralID vendorID:@0xfff1 productID:@0x1234 discriminator:@0x444];
135+
[self waitForExpectations:@[ didFindDevice ] timeout:2 enforceOrder:YES];
136+
137+
XCTAssertTrue([sController stopBrowseForCommissionables]);
138+
139+
// Attempt to use the MTRCommissionableBrowserResult after we stopped browsing
140+
// This used to result in a UAF because BLE_CONNECTION_OBJECT is a void*
141+
// carrying a CBPeripheral without retaining it. When browsing is stopped,
142+
// BleConnectionDelegateImpl releases all cached CBPeripherals.
143+
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithSetupPasscode:@54321 discriminator:@0x444];
144+
[sController setupCommissioningSessionWithDiscoveredDevice:device
145+
payload:payload
146+
newNodeID:@999
147+
error:NULL];
148+
[sController cancelCommissioningForNodeID:@999 error:NULL];
149+
}
150+
151+
- (void)testShutdownBlePowerOffRaceUAF
152+
{
153+
// Attempt a PASE connection over BLE, this will call BleConnectionDelegateImpl::NewConnection()
154+
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithSetupPasscode:@54321 discriminator:@0xb1e];
155+
payload.discoveryCapabilities = MTRDiscoveryCapabilitiesBLE;
156+
NSError * error;
157+
XCTAssertTrue([sController setupCommissioningSessionWithPayload:payload newNodeID:@999 error:&error],
158+
"setupCommissioningSessionWithPayload failed: %@", error);
159+
160+
// Create a race between shutdown and a CBManager callback that used to provoke a UAF.
161+
// Note that on the order of 100 iterations can be necessary to reproduce the crash.
162+
__block atomic_int tasks = 2;
163+
dispatch_semaphore_t done = dispatch_semaphore_create(0);
164+
165+
dispatch_block_t shutdown = ^{
166+
// Shut down the controller. This causes the SetupCodePairer to call
167+
// BleConnectionDelegateImpl::CancelConnection(), then the SetupCodePairer
168+
// is deallocated along with the DeviceCommissioner
169+
[sController shutdown];
170+
sController = nil;
171+
if (atomic_fetch_sub(&tasks, 1) == 1) {
172+
dispatch_semaphore_signal(done);
173+
}
174+
};
175+
dispatch_block_t powerOff = ^{
176+
// Cause CBPeripheralManager to signal a state change that
177+
// triggers a callback to the SetupCodePairer
178+
self.class.mockCoreBluetooth.state = CBManagerStatePoweredOff;
179+
if (atomic_fetch_sub(&tasks, 1) == 1) {
180+
dispatch_semaphore_signal(done);
181+
}
182+
};
183+
184+
dispatch_queue_t pool = dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0);
185+
dispatch_async(pool, shutdown);
186+
dispatch_async(pool, powerOff);
187+
dispatch_wait(done, DISPATCH_TIME_FOREVER);
188+
}
189+
190+
@end
191+
192+
NS_ASSUME_NONNULL_END

0 commit comments

Comments
 (0)