Skip to content

Commit dad8a13

Browse files
Add check for stepSize and stepMode and move the parameters validation at function entry
1 parent 3e81312 commit dad8a13

File tree

1 file changed

+25
-17
lines changed

1 file changed

+25
-17
lines changed

src/app/clusters/level-control/level-control.cpp

+25-17
Original file line numberDiff line numberDiff line change
@@ -942,31 +942,30 @@ static void moveHandler(app::CommandHandler * commandObj, const app::ConcreteCom
942942
app::DataModel::Nullable<uint8_t> rate, chip::Optional<BitMask<OptionsBitmap>> optionsMask,
943943
chip::Optional<BitMask<OptionsBitmap>> optionsOverride)
944944
{
945-
EndpointId endpoint = commandPath.mEndpointId;
946-
CommandId commandId = commandPath.mCommandId;
947-
948-
EmberAfLevelControlState * state = getState(endpoint);
949945
Status status;
950-
app::DataModel::Nullable<uint8_t> currentLevel;
951946
uint8_t difference;
947+
EmberAfLevelControlState * state;
948+
app::DataModel::Nullable<uint8_t> currentLevel;
952949

953-
if (state == nullptr)
950+
EndpointId endpoint = commandPath.mEndpointId;
951+
CommandId commandId = commandPath.mCommandId;
952+
// Validate the received rate and moveMode first.
953+
if ((!rate.IsNull() && (rate.Value() == 0)) || moveMode == MoveModeEnum::kUnknownEnumValue)
954954
{
955-
status = Status::Failure;
955+
status = Status::InvalidCommand;
956956
goto send_default_response;
957957
}
958958

959-
if (!shouldExecuteIfOff(endpoint, commandId, optionsMask, optionsOverride))
959+
state = getState(endpoint);
960+
if (state == nullptr)
960961
{
961-
status = Status::Success;
962+
status = Status::Failure;
962963
goto send_default_response;
963964
}
964965

965-
// Always validate the rate parameter received. A rate of 0 is invalid.
966-
if (!rate.IsNull() && (rate.Value() == 0))
966+
if (!shouldExecuteIfOff(endpoint, commandId, optionsMask, optionsOverride))
967967
{
968-
// Providing rate of 0 is not allowed.
969-
status = Status::InvalidCommand;
968+
status = Status::Success;
970969
goto send_default_response;
971970
}
972971

@@ -1084,14 +1083,22 @@ static void stepHandler(app::CommandHandler * commandObj, const app::ConcreteCom
10841083
uint8_t stepSize, app::DataModel::Nullable<uint16_t> transitionTimeDs,
10851084
chip::Optional<BitMask<OptionsBitmap>> optionsMask, chip::Optional<BitMask<OptionsBitmap>> optionsOverride)
10861085
{
1087-
EndpointId endpoint = commandPath.mEndpointId;
1088-
CommandId commandId = commandPath.mCommandId;
1089-
1090-
EmberAfLevelControlState * state = getState(endpoint);
10911086
Status status;
1087+
EmberAfLevelControlState * state;
10921088
app::DataModel::Nullable<uint8_t> currentLevel;
1089+
1090+
EndpointId endpoint = commandPath.mEndpointId;
1091+
CommandId commandId = commandPath.mCommandId;
10931092
uint8_t actualStepSize = stepSize;
10941093

1094+
// Validate the received stepSize and stepMode first.
1095+
if (stepSize == 0 || stepMode == StepModeEnum::kUnknownEnumValue)
1096+
{
1097+
status = Status::InvalidCommand;
1098+
goto send_default_response;
1099+
}
1100+
1101+
state = getState(endpoint);
10951102
if (state == nullptr)
10961103
{
10971104
status = Status::Failure;
@@ -1153,6 +1160,7 @@ static void stepHandler(app::CommandHandler * commandObj, const app::ConcreteCom
11531160
}
11541161
break;
11551162
default:
1163+
// Should never happen as it is verified at function entry.
11561164
status = Status::InvalidCommand;
11571165
goto send_default_response;
11581166
}

0 commit comments

Comments
 (0)