From 1955106eeaa41e3f9fc1735496cfd287322773e3 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 17 Jul 2023 20:20:00 -0400 Subject: [PATCH] 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. --- .../basic-information/basic-information.cpp | 6 +++++- .../general-commissioning-server.cpp | 19 ++++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/app/clusters/basic-information/basic-information.cpp b/src/app/clusters/basic-information/basic-information.cpp index 3429978fdc20a8..1e0b1b0d9c0e91 100644 --- a/src/app/clusters/basic-information/basic-information.cpp +++ b/src/app/clusters/basic-information/basic-information.cpp @@ -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(location.size()), location.data()); + return CHIP_IM_GLOBAL_STATUS(ConstraintError); + } return DeviceLayer::ConfigurationMgr().StoreCountryCode(location.data(), location.size()); } diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp index 7548fe013a6327..ef66957d754d68 100644 --- a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp @@ -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(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 { @@ -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; }