Skip to content

Commit

Permalink
Add some validation when deserializing node IDs and CATs.
Browse files Browse the repository at this point in the history
  • Loading branch information
bzbarsky-apple committed Aug 31, 2023
1 parent acde7a7 commit f2b9084
Showing 1 changed file with 79 additions and 1 deletion.
80 changes: 79 additions & 1 deletion src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
#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";
static NSString * sLastLocallyUsedNOCKey = @"lastLocallyUsedControllerNOC";
Expand All @@ -32,6 +36,60 @@
[NSString stringWithFormat:@"caseResumptionByResumptionID/%s", [resumptionID base64EncodedStringWithOptions:0].UTF8String];
}

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

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

if ([number compare:@(0)] == NSOrderedAscending) {
return false;
}

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)) {
return false;
}

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 @@ -69,6 +127,11 @@ - (nullable instancetype)initWithController:(MTRDeviceController *)controller
MTR_LOG_ERROR("List of CASE resumption node IDs contains a non-number");
return nil;
}

if (!IsValidNodeIDNumber(value)) {
MTR_LOG_ERROR("Resumption node ID contains invalid value: %@", value);
return nil;
}
}
_nodesWithResumptionInfo = [NSMutableArray arrayWithCapacity:[resumptionNodeList count]];
[_nodesWithResumptionInfo addObjectsFromArray:resumptionNodeList];
Expand Down Expand Up @@ -222,17 +285,32 @@ + (BOOL)supportsSecureCoding
return YES;
}

- (instancetype)initWithCoder:(NSCoder *)decoder
- (nullable instancetype)initWithCoder:(NSCoder *)decoder
{
self = [super init];
if (self == nil) {
return nil;
}

_nodeID = [decoder decodeObjectOfClass:[NSNumber class] forKey:sNodeIDKey];
if (!IsValidNodeIDNumber(_nodeID)) {
MTR_LOG_ERROR("MTRCASESessionResumptionInfo node ID has invalid value: %@", _nodeID);
return nil;
}

_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 f2b9084

Please sign in to comment.