Skip to content

Commit 889f141

Browse files
committed
Surface integrity-check failure when parsing a bad setup payload
A Manual Pairing Code with a wrong Verhoeff check digit causes ManualSetupPayloadParser::populatePayload to return CHIP_ERROR_INTEGRITY_CHECK_FAILED, but the MTR layer was discarding the specific code: -[MTRSetupPayload initWithManualPairingCode:] dropped it on the floor (no error out-param), and downstream call sites (+setupPayloadWithOnboardingPayload:error:, -[MTRManualSetupPayloadParser populatePayload:], -[MTRDeviceController pairDevice:onboardingPayload:error:]) all flattened parse failures to MTRErrorCodeInvalidArgument. Callers had no way to tell "the user typed a bad setup code" apart from any other parse error. Adversarial audit found the QR Code parse path had the same shape: -[MTRSetupPayload initWithQRCode:] has no error out-param, -[MTRQRCodeSetupPayloadParser populatePayload:] discards the specific CHIP_ERROR and rewrites it to MTRErrorCodeInvalidArgument, and the QR branch of +setupPayloadWithOnboardingPayload:error: does the same. A scanner glitch or a typed-in QR string with a bad Base38 character or invalid chunk length was indistinguishable from any other parse failure, so callers stalled the same way. Add NSError**-bearing -initWithManualPairingCode:error: and -initWithQRCode:error: variants that map the underlying CHIP_ERROR via [MTRError errorForCHIPErrorCode:], and route the downstream call sites through them: - -[MTRManualSetupPayloadParser populatePayload:] - -[MTRQRCodeSetupPayloadParser populatePayload:] - +[MTRSetupPayload setupPayloadWithOnboardingPayload:error:] (both the QR and manual branches collapse to a single dispatch) - -[MTRDeviceController pairDevice:onboardingPayload:error:] The existing no-error inits are preserved as thin wrappers for source compatibility. The CHIP_ERROR -> MTRError mappings already exist in MTRError.mm (CHIP_ERROR_INTEGRITY_CHECK_FAILED -> MTRErrorCodeIntegrityCheckFailed, CHIP_ERROR_INVALID_INTEGER_VALUE -> MTRErrorCodeInvalidIntegerValue, CHIP_ERROR_INVALID_STRING_LENGTH -> MTRErrorCodeInvalidStringLength); no new error codes are needed. Adds XCTests against MTRSetupPayloadTests.m pinning the new error shape across both parse paths: - bad Verhoeff check digit -> MTRErrorCodeIntegrityCheckFailed - bad Base38 character -> MTRErrorCodeInvalidIntegerValue - invalid Base38 chunk length -> MTRErrorCodeInvalidStringLength and a MTRPairingBackwardsCompatTests integration pin on the pairDevice:onboardingPayload:error: surface. The existing C++ regression guard (TestManualCode.TestPayloadParser_InvalidEntry) is left intact. NFC commissioning shares the QR parse path (the NFC tag carries a Matter-encoded URL whose payload portion is the same Base38 QR string), so this fix covers NFC payload failures too.
1 parent 6c85140 commit 889f141

12 files changed

Lines changed: 192 additions & 22 deletions

File tree

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2383,11 +2383,8 @@ - (BOOL)pairDevice:(uint64_t)deviceID
23832383

23842384
- (BOOL)pairDevice:(uint64_t)deviceID onboardingPayload:(NSString *)onboardingPayload error:(NSError * __autoreleasing *)error
23852385
{
2386-
auto * setupPayload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload];
2386+
auto * setupPayload = [MTRSetupPayload setupPayloadWithOnboardingPayload:onboardingPayload error:error];
23872387
if (!setupPayload) {
2388-
if (error) {
2389-
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
2390-
}
23912388
return NO;
23922389
}
23932390

src/darwin/Framework/CHIP/MTRManualSetupPayloadParser.mm

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,7 @@ - (id)initWithDecimalStringRepresentation:(NSString *)decimalStringRepresentatio
3333

3434
- (MTRSetupPayload *)populatePayload:(NSError * __autoreleasing *)error
3535
{
36-
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:_decimalStringRepresentation];
37-
if (!payload && error) {
38-
*error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
39-
}
40-
return payload;
36+
return [[MTRSetupPayload alloc] initWithManualPairingCode:_decimalStringRepresentation error:error];
4137
}
4238

4339
@end

src/darwin/Framework/CHIP/MTRQRCodeSetupPayloadParser.mm

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,7 @@ - (id)initWithBase38Representation:(NSString *)base38Representation
3333

3434
- (MTRSetupPayload *)populatePayload:(NSError * __autoreleasing *)error
3535
{
36-
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithQRCode:_base38Representation];
37-
if (!payload && error) {
38-
*error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
39-
}
40-
return payload;
36+
return [[MTRSetupPayload alloc] initWithQRCode:_base38Representation error:error];
4137
}
4238

4339
@end

src/darwin/Framework/CHIP/MTRSetupPayload.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
121121
*/
122122
- (nullable instancetype)initWithPayload:(NSString *)payload MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6));
123123

124+
/**
125+
* Initializes the payload object from the provided Manual Pairing Code string.
126+
*
127+
* Returns nil and populates `error` (if non-nil) if the string is not a valid
128+
* Manual Pairing Code. The returned error is in MTRErrorDomain; in particular,
129+
* a Manual Pairing Code with an incorrect Verhoeff check digit will report
130+
* `MTRErrorCodeIntegrityCheckFailed`, allowing callers to distinguish a
131+
* mistyped setup code from other parse failures.
132+
*/
133+
- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
134+
error:(NSError * __autoreleasing *)error MTR_PROVISIONALLY_AVAILABLE;
135+
124136
/**
125137
* Whether this object represents a concatenated QR Code payload consisting
126138
* of two or more underlying payloads. If YES, then:

src/darwin/Framework/CHIP/MTRSetupPayload.mm

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -269,17 +269,32 @@ - (CHIP_ERROR)initializeFromQRCode:(NSString *)qrCode validate:(BOOL)validate
269269
}
270270

271271
- (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload
272+
{
273+
return [self initWithQRCode:qrCodePayload error:nil];
274+
}
275+
276+
- (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload
277+
error:(NSError * __autoreleasing *)error
272278
{
273279
self = [super init];
274280
CHIP_ERROR err = [self initializeFromQRCode:qrCodePayload validate:YES];
275281
if (err != CHIP_NO_ERROR) {
282+
if (error) {
283+
*error = [MTRError errorForCHIPErrorCode:err];
284+
}
276285
return nil;
277286
}
278287

279288
return self;
280289
}
281290

282291
- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
292+
{
293+
return [self initWithManualPairingCode:manualCode error:nil];
294+
}
295+
296+
- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
297+
error:(NSError * __autoreleasing *)error
283298
{
284299
self = [super init];
285300

@@ -288,10 +303,16 @@ - (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
288303
CHIP_ERROR err = parser.populatePayload(_payload);
289304
if (err != CHIP_NO_ERROR) {
290305
MTR_LOG_ERROR("Failed to parse Manual Pairing Code: %" CHIP_ERROR_FORMAT, err.Format());
306+
if (error) {
307+
*error = [MTRError errorForCHIPErrorCode:err];
308+
}
291309
return nil;
292310
}
293311
if (!_payload.isValidManualCode(chip::PayloadContents::ValidationMode::kConsume)) {
294312
MTR_LOG_ERROR("Invalid Manual Pairing Code");
313+
if (error) {
314+
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
315+
}
295316
return nil;
296317
}
297318

@@ -781,11 +802,15 @@ + (instancetype)new
781802
+ (MTRSetupPayload * _Nullable)setupPayloadWithOnboardingPayload:(NSString *)onboardingPayload
782803
error:(NSError * __autoreleasing *)error
783804
{
784-
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload];
785-
if (!payload && error) {
786-
*error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
805+
// Use the error-bearing inits so we can surface the specific underlying
806+
// CHIP_ERROR (e.g. CHIP_ERROR_INTEGRITY_CHECK_FAILED for a bad Verhoeff
807+
// check digit on a Manual Pairing Code, or CHIP_ERROR_INVALID_STRING_LENGTH /
808+
// CHIP_ERROR_INVALID_INTEGER_VALUE for a malformed QR Code) rather than
809+
// flattening every parse failure to MTRErrorCodeInvalidArgument.
810+
if ([onboardingPayload hasPrefix:@"MT:"]) {
811+
return [[MTRSetupPayload alloc] initWithQRCode:onboardingPayload error:error];
787812
}
788-
return payload;
813+
return [[MTRSetupPayload alloc] initWithManualPairingCode:onboardingPayload error:error];
789814
}
790815

791816
- (nullable NSNumber *)rendezvousInformation

src/darwin/Framework/CHIP/MTRSetupPayload_Internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ MTR_DIRECT_MEMBERS
2828

2929
- (instancetype)initWithSetupPayload:(const chip::SetupPayload &)setupPayload;
3030
- (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload;
31+
- (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload error:(NSError * __autoreleasing *)error;
3132
- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode;
3233

3334
@end

src/darwin/Framework/CHIPTests/MTRPairingBackwardsCompatTests.m

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,25 @@ - (void)test002_PairDeviceWithString
118118
[self waitForExpectations:@[ expectation ] timeout:kPairingTimeoutInSeconds];
119119
}
120120

121+
- (void)test003_PairDeviceWithBadManualPairingCode_SurfacesIntegrityCheckFailed
122+
{
123+
// Integration regression pin: pairing through MTRDeviceController with a
124+
// manual pairing code whose Verhoeff check digit is wrong must surface
125+
// MTRErrorCodeIntegrityCheckFailed end-to-end, not the pre-fix generic
126+
// MTRErrorCodeInvalidArgument. No server app is needed -- this fails at
127+
// parse time before any PASE work is attempted.
128+
__auto_type * controller = [self createControllerOnTestFabric];
129+
XCTAssertNotNil(controller);
130+
131+
NSError * error;
132+
BOOL ok = [controller pairDevice:sDeviceId
133+
onboardingPayload:@"02684354589"
134+
error:&error];
135+
XCTAssertFalse(ok);
136+
XCTAssertNotNil(error);
137+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
138+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
139+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
140+
}
141+
121142
@end

src/darwin/Framework/CHIPTests/MTRSetupPayloadTests.m

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,4 +562,126 @@ - (void)testValidSetupPasscode
562562
XCTAssertTrue([MTRSetupPayload isValidSetupPasscode:[MTRSetupPayload generateRandomSetupPasscode]]);
563563
}
564564

565+
- (void)testManualPairingCode_LastDigitCorruption_ReturnsIntegrityCheckFailed
566+
{
567+
// Regression pin (1/2): malformed setup code with last-digit corruption.
568+
// Take a known-good 21-digit manual pairing code, flip ONLY its last digit
569+
// (the Verhoeff check digit), and confirm the parser rejects the corrupted
570+
// form with MTRErrorCodeIntegrityCheckFailed while still accepting the
571+
// canonical form. Pins the bug where typo-on-the-last-digit was silently
572+
// flattened to MTRErrorCodeInvalidArgument and indistinguishable from any
573+
// other parse failure.
574+
NSString * const validCode = @"641286075300001000016";
575+
NSString * const corruptedCode = @"641286075300001000017"; // last digit 6 -> 7
576+
577+
NSError * validError;
578+
MTRSetupPayload * validPayload = [[MTRSetupPayload alloc] initWithManualPairingCode:validCode error:&validError];
579+
XCTAssertNotNil(validPayload);
580+
XCTAssertNil(validError);
581+
582+
NSError * corruptedError;
583+
MTRSetupPayload * corruptedPayload = [[MTRSetupPayload alloc] initWithManualPairingCode:corruptedCode error:&corruptedError];
584+
XCTAssertNil(corruptedPayload);
585+
XCTAssertNotNil(corruptedError);
586+
XCTAssertEqualObjects(corruptedError.domain, MTRErrorDomain);
587+
XCTAssertEqual(corruptedError.code, MTRErrorCodeIntegrityCheckFailed);
588+
}
589+
590+
- (void)testManualPairingCode_BadCheckDigit_SurfacesIntegrityCheckFailedAcrossAPIs
591+
{
592+
// Regression pin (2/2): integrity-check error surfaces correct MTRError.
593+
// A manual pairing code with a wrong Verhoeff check digit must surface as
594+
// MTRErrorCodeIntegrityCheckFailed (NOT MTRErrorCodeInvalidArgument) on
595+
// every public entry point that parses one. Covers the three surfaces the
596+
// pre-fix code flattened: -initWithManualPairingCode:error:,
597+
// +setupPayloadWithOnboardingPayload:error:, and -[MTRManualSetupPayloadParser
598+
// populatePayload:].
599+
NSString * const badCode = @"02684354589";
600+
601+
{
602+
NSError * error;
603+
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:badCode error:&error];
604+
XCTAssertNil(payload);
605+
XCTAssertNotNil(error);
606+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
607+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
608+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
609+
}
610+
{
611+
NSError * error;
612+
MTRSetupPayload * payload = [MTRSetupPayload setupPayloadWithOnboardingPayload:badCode error:&error];
613+
XCTAssertNil(payload);
614+
XCTAssertNotNil(error);
615+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
616+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
617+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
618+
}
619+
{
620+
MTRManualSetupPayloadParser * parser =
621+
[[MTRManualSetupPayloadParser alloc] initWithDecimalStringRepresentation:badCode];
622+
NSError * error;
623+
MTRSetupPayload * payload = [parser populatePayload:&error];
624+
XCTAssertNil(payload);
625+
XCTAssertNotNil(error);
626+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
627+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
628+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
629+
}
630+
}
631+
632+
- (void)testQRCode_MalformedBase38_SurfacesSpecificErrorAcrossAPIs
633+
{
634+
// Regression pin: the QR-code parse path used to flatten every failure to
635+
// MTRErrorCodeInvalidArgument the same way the manual-code path did, which
636+
// meant a typo or scanner glitch in a QR Code was indistinguishable from a
637+
// structural rejection. A bad Base38 character (here, the '?' which is not
638+
// in the Base38 alphabet) returns CHIP_ERROR_INVALID_INTEGER_VALUE from
639+
// base38Decode and must surface as MTRErrorCodeInvalidIntegerValue (NOT
640+
// MTRErrorCodeInvalidArgument) on every entry point that parses a QR Code:
641+
// -initWithQRCode:error: (internal), +setupPayloadWithOnboardingPayload:error:,
642+
// and -[MTRQRCodeSetupPayloadParser populatePayload:].
643+
NSString * const malformedQRCode = @"MT:M5L90MP500K64J0000?";
644+
645+
{
646+
NSError * error;
647+
MTRSetupPayload * payload = [MTRSetupPayload setupPayloadWithOnboardingPayload:malformedQRCode error:&error];
648+
XCTAssertNil(payload);
649+
XCTAssertNotNil(error);
650+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
651+
XCTAssertEqual(error.code, MTRErrorCodeInvalidIntegerValue);
652+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
653+
}
654+
{
655+
// Strip the "MT:" prefix for the Base38-only parser surface.
656+
MTRQRCodeSetupPayloadParser * parser =
657+
[[MTRQRCodeSetupPayloadParser alloc] initWithBase38Representation:[malformedQRCode substringFromIndex:3]];
658+
NSError * error;
659+
MTRSetupPayload * payload = [parser populatePayload:&error];
660+
XCTAssertNil(payload);
661+
XCTAssertNotNil(error);
662+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
663+
XCTAssertEqual(error.code, MTRErrorCodeInvalidIntegerValue);
664+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
665+
}
666+
}
667+
668+
- (void)testQRCode_TruncatedBase38_SurfacesInvalidStringLength
669+
{
670+
// Companion to the malformed-Base38 case: a Base38 payload whose total
671+
// length is not decomposable into the allowed chunk sizes (2, 4, 5) returns
672+
// CHIP_ERROR_INVALID_STRING_LENGTH from base38Decode. The valid 19-character
673+
// payload below would parse, but appending two extra characters leaves the
674+
// length at 21 = 5+5+5+5+1, with a single trailing character that cannot
675+
// form any allowed chunk. That must surface as MTRErrorCodeInvalidStringLength,
676+
// not the pre-fix MTRErrorCodeInvalidArgument.
677+
NSError * error;
678+
MTRSetupPayload * payload =
679+
[MTRSetupPayload setupPayloadWithOnboardingPayload:@"MT:M5L90MP500K64J00000AB" error:&error];
680+
XCTAssertNil(payload);
681+
XCTAssertNotNil(error);
682+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
683+
XCTAssertEqual(error.code, MTRErrorCodeInvalidStringLength);
684+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
685+
}
686+
565687
@end

third_party/openthread/repo

Submodule repo updated 257 files

third_party/perfetto/repo

Submodule repo updated 4100 files

0 commit comments

Comments
 (0)