Skip to content

Commit b0350f2

Browse files
Enforce length constraint for CountryCode in SetRegulatoryConfig. (#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.
1 parent b3bd8d6 commit b0350f2

File tree

2 files changed

+21
-4
lines changed

2 files changed

+21
-4
lines changed

src/app/clusters/basic-information/basic-information.cpp

+5-1
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,11 @@ CHIP_ERROR BasicAttrAccess::WriteLocation(AttributeValueDecoder & aDecoder)
344344
ReturnErrorOnFailure(aDecoder.Decode(location));
345345

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

349353
return DeviceLayer::ConfigurationMgr().StoreCountryCode(location.data(), location.size());
350354
}

src/app/clusters/general-commissioning-server/general-commissioning-server.cpp

+16-3
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,21 @@ bool emberAfGeneralCommissioningClusterSetRegulatoryConfigCallback(app::CommandH
287287
DeviceControlServer * server = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
288288
Commands::SetRegulatoryConfigResponse::Type response;
289289

290+
auto & countryCode = commandData.countryCode;
291+
bool isValidLength = countryCode.size() == DeviceLayer::ConfigurationManager::kMaxLocationLength;
292+
if (!isValidLength)
293+
{
294+
ChipLogError(Zcl, "Invalid country code: '%.*s'", static_cast<int>(countryCode.size()), countryCode.data());
295+
commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::ConstraintError);
296+
return true;
297+
}
298+
290299
if (commandData.newRegulatoryConfig > RegulatoryLocationTypeEnum::kIndoorOutdoor)
291300
{
292301
response.errorCode = CommissioningErrorEnum::kValueOutsideRange;
293-
response.debugText = commandData.countryCode;
302+
// TODO: How does using the country code in debug text make sense, if
303+
// the real issue is the newRegulatoryConfig value?
304+
response.debugText = countryCode;
294305
}
295306
else
296307
{
@@ -304,11 +315,13 @@ bool emberAfGeneralCommissioningClusterSetRegulatoryConfigCallback(app::CommandH
304315
if ((locationCapability != to_underlying(RegulatoryLocationTypeEnum::kIndoorOutdoor)) && (location != locationCapability))
305316
{
306317
response.errorCode = CommissioningErrorEnum::kValueOutsideRange;
307-
response.debugText = commandData.countryCode;
318+
// TODO: How does using the country code in debug text make sense, if
319+
// the real issue is the newRegulatoryConfig value?
320+
response.debugText = countryCode;
308321
}
309322
else
310323
{
311-
CheckSuccess(server->SetRegulatoryConfig(location, commandData.countryCode), Failure);
324+
CheckSuccess(server->SetRegulatoryConfig(location, countryCode), Failure);
312325
Breadcrumb::Set(commandPath.mEndpointId, commandData.breadcrumb);
313326
response.errorCode = CommissioningErrorEnum::kOk;
314327
}

0 commit comments

Comments
 (0)