[SetUpCodePairer] Ensure the payload is a valid payload (#18606)

* [SetUpCodePairer] Ensure the payload is a valid payload

* Hardcode some constraints about the payload content

src/controller/SetUpCodePairer.cpp

@@ -44,8 +44,16 @@ CHIP_ERROR SetUpCodePairer::PairDevice(NodeId remoteId, const char * setUpCode,
  mConnectionType = commission;
 
  bool isQRCode = strncmp(setUpCode, kQRCodePrefix, strlen(kQRCodePrefix)) == 0;
- ReturnErrorOnFailure(isQRCode ? QRCodeSetupPayloadParser(setUpCode).populatePayload(payload)
- : ManualSetupPayloadParser(setUpCode).populatePayload(payload));
+ if (isQRCode)
+ {
+ ReturnErrorOnFailure(QRCodeSetupPayloadParser(setUpCode).populatePayload(payload));
+ VerifyOrReturnError(payload.isValidQRCodePayload(), CHIP_ERROR_INVALID_ARGUMENT);
+ }
+ else
+ {
+ ReturnErrorOnFailure(ManualSetupPayloadParser(setUpCode).populatePayload(payload));
+ VerifyOrReturnError(payload.isValidManualCode(), CHIP_ERROR_INVALID_ARGUMENT);
+ }
 
  mRemoteId = remoteId;
  mSetUpPINCode = payload.setUpPINCode;

src/setup_payload/ManualSetupPayloadParser.cpp

@@ -131,6 +131,12 @@ CHIP_ERROR ManualSetupPayloadParser::populatePayload(SetupPayload & outPayload)
  return result;
  }
 
+ // First digit of '8' or '9' would be invalid for v1 and would indicate new format (e.g. version 2)
+ if (chunk1 == 8 || chunk1 == 9)
+ {
+ return CHIP_ERROR_INVALID_ARGUMENT;
+ }
+
  bool isLongCode = ((chunk1 >> kManualSetupChunk1VidPidPresentBitPos) & 1) == 1;
  result = CheckCodeLengthValidity(representationWithoutCheckDigit, isLongCode);
  if (result != CHIP_NO_ERROR)

src/setup_payload/SetupPayload.cpp

@@ -27,6 +27,7 @@
 #include <lib/core/CHIPTLV.h>
 #include <lib/core/CHIPTLVData.hpp>
 #include <lib/core/CHIPTLVUtilities.hpp>
+#include <lib/core/CHIPVendorIdentifiers.hpp>
 #include <lib/support/CodeUtils.h>
 #include <utility>
 
@@ -49,6 +50,7 @@ bool SetupPayload::IsVendorTag(uint8_t tag)
 // `vendor_id` and `product_id` are allowed all of uint16_t
 bool PayloadContents::isValidQRCodePayload() const
 {
+ // 3-bit value specifying the QR code payload version.
  if (version >= 1 << kVersionFieldLengthInBits)
  {
  return false;
@@ -59,6 +61,17 @@ bool PayloadContents::isValidQRCodePayload() const
  return false;
  }
 
+ // Device Commissioning Flow
+ // 0: Standard commissioning flow: such a device, when uncommissioned, always enters commissioning mode upon power-up, subject
+ // to the rules in [ref_Announcement_Commencement]. 1: User-intent commissioning flow: user action required to enter
+ // commissioning mode. 2: Custom commissioning flow: interaction with a vendor-specified means is needed before commissioning.
+ // 3: Reserved
+ if (commissioningFlow != CommissioningFlow::kStandard && commissioningFlow != CommissioningFlow::kUserActionRequired &&
+ commissioningFlow != CommissioningFlow::kCustom)
+ {
+ return false;
+ }
+
  chip::RendezvousInformationFlags allvalid(RendezvousInformationFlag::kBLE, RendezvousInformationFlag::kOnNetwork,
  RendezvousInformationFlag::kSoftAP);
  if (!rendezvousInformation.HasOnly(allvalid))
@@ -76,12 +89,7 @@ bool PayloadContents::isValidQRCodePayload() const
  return false;
  }
 
- if (version == 0 && !rendezvousInformation.HasAny(allvalid) && discriminator == 0 && setUpPINCode == 0)
- {
- return false;
- }
-
- return true;
+ return CheckPayloadCommonConstraints();
 }
 
 bool PayloadContents::isValidManualCode() const
@@ -100,7 +108,37 @@ bool PayloadContents::isValidManualCode() const
  return false;
  }
 
- if (setUpPINCode == 0)
+ return CheckPayloadCommonConstraints();
+}
+
+bool PayloadContents::CheckPayloadCommonConstraints() const
+{
+ // A version not equal to 0 would be invalid for v1 and would indicate new format (e.g. version 2)
+ if (version != 0)
+ {
+ return false;
+ }
+
+ // SHALL be restricted to the values 0x0000001 to 0x5F5E0FE (00000001 to 99999998 in decimal), excluding the invalid Passcode
+ // values.
+ if (setUpPINCode < 0x0000001 || setUpPINCode > 0x5F5E0FE || setUpPINCode == 11111111 || setUpPINCode == 22222222 ||
+ setUpPINCode == 33333333 || setUpPINCode == 44444444 || setUpPINCode == 55555555 || setUpPINCode == 66666666 ||
+ setUpPINCode == 77777777 || setUpPINCode == 88888888 || setUpPINCode == 12345678 || setUpPINCode == 87654321)
+ {
+ return false;
+ }
+
+ // VendorID after TestVendor4 are reserved.
+ if (vendorID > VendorId::TestVendor4)
+ {
+ return false;
+ }
+
+ // A value of 0x0000 SHALL NOT be assigned to a product since Product ID = 0x0000 is used for these specific cases:
+ // * To announce an anonymized Product ID as part of device discovery
+ // * To indicate an OTA software update file applies to multiple Product IDs equally.
+ // * To avoid confusion when presenting the Onboarding Payload for ECM with multiple nodes
+ if (productID == 0 && vendorID != 0)
  {
  return false;
  }

src/setup_payload/SetupPayload.h

@@ -119,6 +119,9 @@ struct PayloadContents
  bool isValidManualCode() const;
  bool isShortDiscriminator = false;
  bool operator==(PayloadContents & input) const;
+
+private:
+ bool CheckPayloadCommonConstraints() const;
 };
 
 enum optionalQRCodeInfoType

src/setup_payload/tests/TestHelpers.h

@@ -39,13 +39,13 @@ const uint32_t kOptionalDefaultIntValue = 12;
 constexpr char kSerialNumberDefaultStringValue[] = "123456789";
 const uint32_t kSerialNumberDefaultUInt32Value = 123456789;
 
-constexpr const char * kDefaultPayloadQRCode = "MT:R5L90MP500K64J00000";
+constexpr const char * kDefaultPayloadQRCode = "MT:M5L90MP500K64J00000";
 
 inline SetupPayload GetDefaultPayload()
 {
  SetupPayload payload;
 
- payload.version = 5;
+ payload.version = 0;
  payload.vendorID = 12;
  payload.productID = 1;
  payload.commissioningFlow = CommissioningFlow::kStandard;
@@ -132,14 +132,16 @@ inline bool CompareBinary(SetupPayload & payload, std::string & expectedBinary)
  return (expectedBinary == resultBinary);
 }
 
-inline bool CheckWriteRead(SetupPayload & inPayload)
+inline bool CheckWriteRead(SetupPayload & inPayload, bool allowInvalidPayload = false)
 {
  SetupPayload outPayload;
  std::string result;
 
  uint8_t optionalInfo[kDefaultBufferSizeInBytes];
  memset(optionalInfo, 0xFF, sizeof(optionalInfo));
- QRCodeSetupPayloadGenerator(inPayload).payloadBase38Representation(result, optionalInfo, sizeof(optionalInfo));
+ auto generator = QRCodeSetupPayloadGenerator(inPayload);
+ generator.SetAllowInvalidPayload(allowInvalidPayload);
+ generator.payloadBase38Representation(result, optionalInfo, sizeof(optionalInfo));
 
  outPayload = {};
  QRCodeSetupPayloadParser(result).populatePayload(outPayload);

src/setup_payload/tests/TestManualCode.cpp

@@ -38,10 +38,11 @@ using namespace chip;
 
 namespace {
 
-bool CheckGenerator(const PayloadContents & payload, std::string expectedResult)
+bool CheckGenerator(const PayloadContents & payload, std::string expectedResult, bool allowInvalidPayload = false)
 {
  std::string result;
  ManualSetupPayloadGenerator generator(payload);
+ generator.SetAllowInvalidPayload(allowInvalidPayload);
  generator.payloadDecimalStringRepresentation(result);
 
  if (!expectedResult.empty())
@@ -62,7 +63,7 @@ bool CheckGenerator(const PayloadContents & payload, std::string expectedResult)
 PayloadContents GetDefaultPayload()
 {
  PayloadContents payload;
- payload.setUpPINCode = 123456780;
+ payload.setUpPINCode = 12345679;
  payload.discriminator = 2560;
 
  return payload;
@@ -72,7 +73,7 @@ void TestDecimalRepresentation_PartialPayload(nlTestSuite * inSuite, void * inCo
 {
  PayloadContents payload = GetDefaultPayload();
 
- std::string expectedResult = "2361087535";
+ std::string expectedResult = "2412950753";
 
  NL_TEST_ASSERT(inSuite, CheckGenerator(payload, expectedResult));
 }
@@ -82,7 +83,7 @@ void TestDecimalRepresentation_PartialPayload_RequiresCustomFlow(nlTestSuite * i
  PayloadContents payload = GetDefaultPayload();
  payload.commissioningFlow = CommissioningFlow::kCustom;
 
- std::string expectedResult = "63610875350000000000";
+ std::string expectedResult = "64129507530000000000";
 
  NL_TEST_ASSERT(inSuite, CheckGenerator(payload, expectedResult));
 }
@@ -94,7 +95,7 @@ void TestDecimalRepresentation_FullPayloadWithZeros(nlTestSuite * inSuite, void
  payload.vendorID = 1;
  payload.productID = 1;
 
- std::string expectedResult = "63610875350000100001";
+ std::string expectedResult = "64129507530000100001";
 
  NL_TEST_ASSERT(inSuite, CheckGenerator(payload, expectedResult));
 }
@@ -106,7 +107,7 @@ void TestDecimalRepresentation_FullPayloadWithoutZeros(nlTestSuite * inSuite, vo
  payload.vendorID = 45367;
  payload.productID = 14526;
 
- std::string expectedResult = "63610875354536714526";
+ std::string expectedResult = "64129507534536714526";
 
  NL_TEST_ASSERT(inSuite, CheckGenerator(payload, expectedResult));
 }
@@ -117,7 +118,7 @@ void TestDecimalRepresentation_FullPayloadWithoutZeros_DoesNotRequireCustomFlow(
  payload.vendorID = 45367;
  payload.productID = 14526;
 
- std::string expectedResult = "2361087535";
+ std::string expectedResult = "2412950753";
 
  NL_TEST_ASSERT(inSuite, CheckGenerator(payload, expectedResult));
 }
@@ -144,7 +145,7 @@ void TestDecimalRepresentation_AllOnes(nlTestSuite * inSuite, void * inContext)
 
  std::string expectedResult = "76553581916553565535";
 
- NL_TEST_ASSERT(inSuite, CheckGenerator(payload, expectedResult));
+ NL_TEST_ASSERT(inSuite, CheckGenerator(payload, expectedResult, /*allowInvalidPayload*/ true));
 }
 
 void TestDecimalRepresentation_InvalidPayload(nlTestSuite * inSuite, void * inContext)

src/setup_payload/tests/TestQRCode.cpp

@@ -91,15 +91,14 @@ void TestMaximumValues(nlTestSuite * inSuite, void * inContext)
  inPayload.discriminator = static_cast<uint16_t>((1 << kPayloadDiscriminatorFieldLengthInBits) - 1);
  inPayload.setUpPINCode = static_cast<uint32_t>((1 << kSetupPINCodeFieldLengthInBits) - 1);
 
- NL_TEST_ASSERT(inSuite, inPayload.isValidQRCodePayload());
- NL_TEST_ASSERT(inSuite, CheckWriteRead(inPayload));
+ NL_TEST_ASSERT(inSuite, CheckWriteRead(inPayload, /* allowInvalidPayload */ true));
 }
 
 void TestPayloadByteArrayRep(nlTestSuite * inSuite, void * inContext)
 {
  SetupPayload payload = GetDefaultPayload();
 
- string expected = " 0000 000000000000000100000000000 000010000000 00000001 00 0000000000000001 0000000000001100 101";
+ string expected = " 0000 000000000000000100000000000 000010000000 00000001 00 0000000000000001 0000000000001100 000";
  NL_TEST_ASSERT(inSuite, CompareBinary(payload, expected));
 }