Skip to content

Commit 8fb929d

Browse files
committed
Surface integrity-check failure when parsing a bad manual pairing code
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. Add an NSError**-bearing -initWithManualPairingCode:error: variant that maps the underlying CHIP_ERROR via [MTRError errorForCHIPErrorCode:], and route the three downstream call sites through it. The existing no-error initWithManualPairingCode: is preserved as a thin wrapper. The CHIP_ERROR_INTEGRITY_CHECK_FAILED -> MTRErrorCodeIntegrityCheckFailed mapping already exists in MTRError.mm; no new error code is needed. Adds two focused regression tests to MTRSetupPayloadTests.m: 1. last-digit corruption of a known-good manual pairing code surfaces MTRErrorCodeIntegrityCheckFailed (and the canonical form still parses). 2. bad-Verhoeff code surfaces MTRErrorCodeIntegrityCheckFailed (and not MTRErrorCodeInvalidArgument) on all three public parse entry points. The existing C++ regression guard (TestManualCode.TestPayloadParser_InvalidEntry) is left intact.
1 parent adba29b commit 8fb929d

5 files changed

Lines changed: 100 additions & 9 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/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: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,12 @@ - (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload
280280
}
281281

282282
- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
283+
{
284+
return [self initWithManualPairingCode:manualCode error:nil];
285+
}
286+
287+
- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
288+
error:(NSError * __autoreleasing *)error
283289
{
284290
self = [super init];
285291

@@ -288,10 +294,16 @@ - (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
288294
CHIP_ERROR err = parser.populatePayload(_payload);
289295
if (err != CHIP_NO_ERROR) {
290296
MTR_LOG_ERROR("Failed to parse Manual Pairing Code: %" CHIP_ERROR_FORMAT, err.Format());
297+
if (error) {
298+
*error = [MTRError errorForCHIPErrorCode:err];
299+
}
291300
return nil;
292301
}
293302
if (!_payload.isValidManualCode(chip::PayloadContents::ValidationMode::kConsume)) {
294303
MTR_LOG_ERROR("Invalid Manual Pairing Code");
304+
if (error) {
305+
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
306+
}
295307
return nil;
296308
}
297309

@@ -781,6 +793,13 @@ + (instancetype)new
781793
+ (MTRSetupPayload * _Nullable)setupPayloadWithOnboardingPayload:(NSString *)onboardingPayload
782794
error:(NSError * __autoreleasing *)error
783795
{
796+
// For Manual Pairing Codes use the error-bearing init so we can surface
797+
// the underlying CHIP_ERROR (e.g. CHIP_ERROR_INTEGRITY_CHECK_FAILED for a
798+
// bad Verhoeff check digit) rather than flattening to InvalidArgument.
799+
if (![onboardingPayload hasPrefix:@"MT:"]) {
800+
return [[MTRSetupPayload alloc] initWithManualPairingCode:onboardingPayload error:error];
801+
}
802+
784803
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload];
785804
if (!payload && error) {
786805
*error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];

src/darwin/Framework/CHIPTests/MTRSetupPayloadTests.m

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,4 +562,71 @@ - (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+
565632
@end

0 commit comments

Comments
 (0)