Skip to content

Commit

Permalink
Add decode-time validation for node id and CAT values.
Browse files Browse the repository at this point in the history
  • Loading branch information
bzbarsky-apple committed Aug 31, 2023
1 parent 45052f3 commit 5fe76db
Showing 1 changed file with 46 additions and 3 deletions.
49 changes: 46 additions & 3 deletions src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
#include "MTRDeviceControllerDataStore.h"
#import "MTRLogging_Internal.h"

#include <lib/core/CASEAuthTag.h>
#include <lib/core/NodeId.h>
#include <lib/support/SafeInt.h>

// FIXME: Are these good key strings? https://github.com/project-chip/connectedhomeip/issues/28973
static NSString * sResumptionNodeListKey = @"caseResumptionNodeList";
Expand All @@ -34,20 +36,29 @@
[NSString stringWithFormat:@"caseResumptionByResumptionID/%s", [resumptionID base64EncodedStringWithOptions:0].UTF8String];
}

static bool IsValidNodeIDNumber(NSNumber * _Nullable number)
static bool IsUnsignedIntegerNumber(NSNumber * _Nullable number)
{
if (number == nil) {
return false;
}

// Not sure how to check for the number being an integer.

// Node IDs cannot be negative.
if ([number compare:@(0)] == NSOrderedAscending) {
return false;
}

// Validate that this is a valid operational ID, not some garbage 64-bit
return true;
}

static bool IsValidNodeIDNumber(NSNumber * _Nullable number)
{
// Node IDs cannot be negative.
if (!IsUnsignedIntegerNumber(number)) {
return false;
}

// Validate that this is a valid operational ID, not some garbage unsigned
// int value that can't be a node id.
uint64_t unsignedValue = number.unsignedLongLongValue;
if (!chip::IsOperationalNodeId(unsignedValue)) {
Expand All @@ -57,6 +68,28 @@ static bool IsValidNodeIDNumber(NSNumber * _Nullable number)
return true;
}

static bool IsValidCATNumber(NSNumber * _Nullable number)
{
// CATs cannot be negative.
if (!IsUnsignedIntegerNumber(number)) {
return false;
}

// Validate that this is a valid CAT value and, not some garbage unsigned int
// value that can't be a CAT.
uint64_t unsignedValue = number.unsignedLongLongValue;
if (!chip::CanCastTo<chip::CASEAuthTag>(unsignedValue)) {
return false;
}

auto tag = static_cast<chip::CASEAuthTag>(unsignedValue);
if (!chip::IsValidCASEAuthTag(tag)) {
return false;
}

return true;
}

@implementation MTRDeviceControllerDataStore {
id<MTRDeviceControllerStorageDelegate> _storageDelegate;
dispatch_queue_t _storageDelegateQueue;
Expand Down Expand Up @@ -268,6 +301,16 @@ - (nullable instancetype)initWithCoder:(NSCoder *)decoder
_resumptionID = [decoder decodeObjectOfClass:[NSData class] forKey:sResumptionIDKey];
_sharedSecret = [decoder decodeObjectOfClass:[NSData class] forKey:sSharedSecretKey];
auto caseAuthenticatedTagArray = [decoder decodeArrayOfObjectsOfClass:[NSNumber class] forKey:sCATsKey];

for (NSNumber * value in caseAuthenticatedTagArray) {
if (!IsValidCATNumber(value)) {
MTR_LOG_ERROR("MTRCASESessionResumptionInfo CASE tag has invalid value: %@", value);
return nil;
}

// Range-checking will be done when we try to convert the set to CATValues.
}

_caseAuthenticatedTags = [NSSet setWithArray:caseAuthenticatedTagArray];
return self;
}
Expand Down

0 comments on commit 5fe76db

Please sign in to comment.