From 73323692b916e92fca5affab0f0bf950761a4050 Mon Sep 17 00:00:00 2001 From: Edwin Wu Date: Mon, 13 Jul 2026 00:01:03 -0700 Subject: [PATCH] Fix potential crash in BLE connection when receiving short data packets. PiperOrigin-RevId: 946827334 --- .../apple/Mediums/BLE/GNCMBleConnection.m | 6 +++- .../apple/Mediums/BLE/GNCMBleUtils.mm | 5 ++++ .../Mediums/BLE/Tests/GNCMBleConnectionTest.m | 29 +++++++++++++++++++ .../Mediums/BLE/Tests/GNCMBleUtilsTest.m | 12 ++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/internal/platform/implementation/apple/Mediums/BLE/GNCMBleConnection.m b/internal/platform/implementation/apple/Mediums/BLE/GNCMBleConnection.m index 7d41ebe0..9f96b95d 100644 --- a/internal/platform/implementation/apple/Mediums/BLE/GNCMBleConnection.m +++ b/internal/platform/implementation/apple/Mediums/BLE/GNCMBleConnection.m @@ -140,7 +140,11 @@ NS_ASSUME_NONNULL_BEGIN return; } - if (![[data subdataWithRange:NSMakeRange(0, prefixLength)] isEqual:_serviceIDHash]) { + // IntroductionFrame.service_id_hash. We MUST bounds-check before + // -subdataWithRange:, otherwise a short follow-up packet throws + // NSRangeException on CoreBluetooth's dispatch queue -> objc_terminate. + if (data.length < prefixLength || + ![[data subdataWithRange:NSMakeRange(0, prefixLength)] isEqual:_serviceIDHash]) { GNCLoggerInfo(@"[NEARBY] Input stream: Received wrong data packet and discarded"); return; } diff --git a/internal/platform/implementation/apple/Mediums/BLE/GNCMBleUtils.mm b/internal/platform/implementation/apple/Mediums/BLE/GNCMBleUtils.mm index c582ff11..fda04cf6 100644 --- a/internal/platform/implementation/apple/Mediums/BLE/GNCMBleUtils.mm +++ b/internal/platform/implementation/apple/Mediums/BLE/GNCMBleUtils.mm @@ -78,6 +78,11 @@ NSData *_Nullable GNCMParseBLEFramesIntroductionPacket(NSData *_Nullable data) { ::location::nearby::mediums::SocketVersion::V2 && socket_control_frame.introduction().has_service_id_hash()) { std::string service_id_hash = socket_control_frame.introduction().service_id_hash(); + // service_id_hash is attacker-supplied; clamp to the protocol-defined + // 3-byte length so it cannot be used to inflate prefixLength downstream. + if (service_id_hash.size() != GNCMBleAdvertisementLengthServiceIDHash) { + return nil; + } return [NSData dataWithBytes:service_id_hash.data() length:service_id_hash.length()]; } } diff --git a/internal/platform/implementation/apple/Mediums/BLE/Tests/GNCMBleConnectionTest.m b/internal/platform/implementation/apple/Mediums/BLE/Tests/GNCMBleConnectionTest.m index a382ee08..fc02cec0 100644 --- a/internal/platform/implementation/apple/Mediums/BLE/Tests/GNCMBleConnectionTest.m +++ b/internal/platform/implementation/apple/Mediums/BLE/Tests/GNCMBleConnectionTest.m @@ -241,4 +241,33 @@ static const NSTimeInterval kTimeout = 1.0; [self waitForExpectationsWithTimeout:kTimeout handler:nil]; } +- (void)testReceiveShortDataPacketAfterIntro { + _connection = [GNCMBleConnection connectionWithSocket:(GNSSocket *)_fakeSocket + serviceID:nil + expectedIntroPacket:YES + callbackQueue:_callbackQueue]; + + NSData *introPacket = GNCMGenerateBLEFramesIntroductionPacket(GNCMServiceIDHash(kServiceID)); + + // Receive the intro packet first to set `_serviceIDHash`. + [_fakeSocket simulateSocketDidReceiveData:introPacket]; + + // Receive a data packet that is shorter than the service ID hash length. + // This should not crash; it should just be discarded. + NSData *shortPacket = [@"ab" dataUsingEncoding:NSUTF8StringEncoding]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Payload handler not called"]; + expectation.inverted = YES; + + GNCMConnectionHandlers *handlers = [[GNCMConnectionHandlers alloc] init]; + handlers.payloadHandler = ^(NSData *data) { + [expectation fulfill]; + }; + _connection.connectionHandlers = handlers; + + [_fakeSocket simulateSocketDidReceiveData:shortPacket]; + + [self waitForExpectationsWithTimeout:kTimeout handler:nil]; +} + @end diff --git a/internal/platform/implementation/apple/Mediums/BLE/Tests/GNCMBleUtilsTest.m b/internal/platform/implementation/apple/Mediums/BLE/Tests/GNCMBleUtilsTest.m index 11ca0c50..480d0071 100644 --- a/internal/platform/implementation/apple/Mediums/BLE/Tests/GNCMBleUtilsTest.m +++ b/internal/platform/implementation/apple/Mediums/BLE/Tests/GNCMBleUtilsTest.m @@ -45,6 +45,18 @@ static const NSTimeInterval kWaitForConnectionTimeout = 6.0; // Allow for the 5 XCTAssertEqualObjects(parsedHash, serviceIDHash); } +- (void)testParseBLEFramesIntroductionPacketFailure_InvalidHashLength { + // Too long hash (4 bytes, protocol expects 3 bytes) + NSData *longHash = [@"1234" dataUsingEncoding:NSUTF8StringEncoding]; + NSData *longPacket = GNCMGenerateBLEFramesIntroductionPacket(longHash); + XCTAssertNil(GNCMParseBLEFramesIntroductionPacket(longPacket)); + + // Too short hash (2 bytes, protocol expects 3 bytes) + NSData *shortHash = [@"12" dataUsingEncoding:NSUTF8StringEncoding]; + NSData *shortPacket = GNCMGenerateBLEFramesIntroductionPacket(shortHash); + XCTAssertNil(GNCMParseBLEFramesIntroductionPacket(shortPacket)); +} + - (void)testParseBLEFramesIntroductionPacketFailure_NilData { NSData *parsedHash = GNCMParseBLEFramesIntroductionPacket(nil); XCTAssertNil(parsedHash);