mirror of
https://github.com/kidfromjupiter/nearby.git
synced 2026-09-14 14:46:12 -04:00
Fix potential crash in BLE connection when receiving short data packets.
PiperOrigin-RevId: 946827334
This commit is contained in:
committed by
Copybara-Service
parent
2f3e7beb78
commit
73323692b9
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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()];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user