Skip to content

Commit

Permalink
Enforce length constraint for CountryCode in SetRegulatoryConfig. (pr…
Browse files Browse the repository at this point in the history
…oject-chip#27949)

We were not checking the length (which must be 2), so would allow 1-char or
0-char values.

Also aligns the exact logic with the Location attribute write code and adds some
error logging.
  • Loading branch information
bzbarsky-apple authored and erwinpan1 committed Jul 21, 2023
1 parent 171e78a commit 089c2ea
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
6 changes: 5 additions & 1 deletion src/app/clusters/basic-information/basic-information.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,11 @@ CHIP_ERROR BasicAttrAccess::WriteLocation(AttributeValueDecoder & aDecoder)
ReturnErrorOnFailure(aDecoder.Decode(location));

bool isValidLength = location.size() == DeviceLayer::ConfigurationManager::kMaxLocationLength;
VerifyOrReturnError(isValidLength, StatusIB(Protocols::InteractionModel::Status::InvalidValue).ToChipError());
if (!isValidLength)
{
ChipLogError(Zcl, "Invalid country code: '%.*s'", static_cast<int>(location.size()), location.data());
return CHIP_IM_GLOBAL_STATUS(ConstraintError);
}

return DeviceLayer::ConfigurationMgr().StoreCountryCode(location.data(), location.size());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,21 @@ bool emberAfGeneralCommissioningClusterSetRegulatoryConfigCallback(app::CommandH
DeviceControlServer * server = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
Commands::SetRegulatoryConfigResponse::Type response;

auto & countryCode = commandData.countryCode;
bool isValidLength = countryCode.size() == DeviceLayer::ConfigurationManager::kMaxLocationLength;
if (!isValidLength)
{
ChipLogError(Zcl, "Invalid country code: '%.*s'", static_cast<int>(countryCode.size()), countryCode.data());
commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::ConstraintError);
return true;
}

if (commandData.newRegulatoryConfig > RegulatoryLocationTypeEnum::kIndoorOutdoor)
{
response.errorCode = CommissioningErrorEnum::kValueOutsideRange;
response.debugText = commandData.countryCode;
// TODO: How does using the country code in debug text make sense, if
// the real issue is the newRegulatoryConfig value?
response.debugText = countryCode;
}
else
{
Expand All @@ -304,11 +315,13 @@ bool emberAfGeneralCommissioningClusterSetRegulatoryConfigCallback(app::CommandH
if ((locationCapability != to_underlying(RegulatoryLocationTypeEnum::kIndoorOutdoor)) && (location != locationCapability))
{
response.errorCode = CommissioningErrorEnum::kValueOutsideRange;
response.debugText = commandData.countryCode;
// TODO: How does using the country code in debug text make sense, if
// the real issue is the newRegulatoryConfig value?
response.debugText = countryCode;
}
else
{
CheckSuccess(server->SetRegulatoryConfig(location, commandData.countryCode), Failure);
CheckSuccess(server->SetRegulatoryConfig(location, countryCode), Failure);
Breadcrumb::Set(commandPath.mEndpointId, commandData.breadcrumb);
response.errorCode = CommissioningErrorEnum::kOk;
}
Expand Down

0 comments on commit 089c2ea

Please sign in to comment.