libpldm: Fix validation in decode_get_firmware_parameters_resp
If the completion code is not PLDM_SUCCESS, typically the payload
just has the completion code. Correct the validation checks to
address this.
Tested: Updated the test cases and tests passed.
Signed-off-by: Tom Joseph <rushtotom@gmail.com>
Change-Id: I27f67c070140693ec0c60f8d79fa2fff96b3cc93
diff --git a/libpldm/firmware_update.c b/libpldm/firmware_update.c
index 12043a2..5427c95 100644
--- a/libpldm/firmware_update.c
+++ b/libpldm/firmware_update.c
@@ -450,10 +450,15 @@
if (msg == NULL || resp_data == NULL ||
active_comp_image_set_ver_str == NULL ||
pending_comp_image_set_ver_str == NULL ||
- comp_parameter_table == NULL) {
+ comp_parameter_table == NULL || !payload_length) {
return PLDM_ERROR_INVALID_DATA;
}
+ resp_data->completion_code = msg->payload[0];
+ if (PLDM_SUCCESS != resp_data->completion_code) {
+ return PLDM_SUCCESS;
+ }
+
if (payload_length < sizeof(struct pldm_get_firmware_parameters_resp)) {
return PLDM_ERROR_INVALID_LENGTH;
}
@@ -461,12 +466,6 @@
struct pldm_get_firmware_parameters_resp *response =
(struct pldm_get_firmware_parameters_resp *)msg->payload;
- resp_data->completion_code = response->completion_code;
-
- if (PLDM_SUCCESS != resp_data->completion_code) {
- return PLDM_SUCCESS;
- }
-
if (!is_string_type_valid(
response->active_comp_image_set_ver_str_type) ||
(response->active_comp_image_set_ver_str_len == 0)) {
diff --git a/libpldm/tests/libpldm_firmware_update_test.cpp b/libpldm/tests/libpldm_firmware_update_test.cpp
index ca2b1a4..bb47742 100644
--- a/libpldm/tests/libpldm_firmware_update_test.cpp
+++ b/libpldm/tests/libpldm_firmware_update_test.cpp
@@ -1051,10 +1051,8 @@
TEST(GetFirmwareParameters, decodeResponseErrorCompletionCode)
{
- constexpr std::array<uint8_t,
- hdrSize + sizeof(pldm_get_firmware_parameters_resp)>
- getFwParamsResponse{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ constexpr std::array<uint8_t, hdrSize + sizeof(uint8_t)>
+ getFwParamsResponse{0x00, 0x00, 0x00, 0x01};
auto responseMsg =
reinterpret_cast<const pldm_msg*>(getFwParamsResponse.data());
@@ -1088,39 +1086,45 @@
variable_field outCompParameterTable{};
rc = decode_get_firmware_parameters_resp(
- nullptr, invalidGetFwParamsResponse1.size(), &outResp,
+ nullptr, invalidGetFwParamsResponse1.size() - hdrSize, &outResp,
&outActiveCompImageSetVersion, &outPendingCompImageSetVersion,
&outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
rc = decode_get_firmware_parameters_resp(
- responseMsg, invalidGetFwParamsResponse1.size(), nullptr,
+ responseMsg, invalidGetFwParamsResponse1.size() - hdrSize, nullptr,
&outActiveCompImageSetVersion, &outPendingCompImageSetVersion,
&outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
rc = decode_get_firmware_parameters_resp(
- responseMsg, invalidGetFwParamsResponse1.size(), &outResp, nullptr,
- &outPendingCompImageSetVersion, &outCompParameterTable);
+ responseMsg, invalidGetFwParamsResponse1.size() - hdrSize, &outResp,
+ nullptr, &outPendingCompImageSetVersion, &outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
rc = decode_get_firmware_parameters_resp(
- responseMsg, invalidGetFwParamsResponse1.size(), &outResp,
+ responseMsg, invalidGetFwParamsResponse1.size() - hdrSize, &outResp,
&outActiveCompImageSetVersion, nullptr, &outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
rc = decode_get_firmware_parameters_resp(
- responseMsg, invalidGetFwParamsResponse1.size(), &outResp,
+ responseMsg, invalidGetFwParamsResponse1.size() - hdrSize, &outResp,
&outActiveCompImageSetVersion, &outPendingCompImageSetVersion, nullptr);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
rc = decode_get_firmware_parameters_resp(
responseMsg, 0, &outResp, &outActiveCompImageSetVersion,
&outPendingCompImageSetVersion, &outCompParameterTable);
+ EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+
+ rc = decode_get_firmware_parameters_resp(
+ responseMsg, invalidGetFwParamsResponse1.size() - 1 - hdrSize, &outResp,
+ &outActiveCompImageSetVersion, &outPendingCompImageSetVersion,
+ &outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
rc = decode_get_firmware_parameters_resp(
- responseMsg, invalidGetFwParamsResponse1.size(), &outResp,
+ responseMsg, invalidGetFwParamsResponse1.size() - hdrSize, &outResp,
&outActiveCompImageSetVersion, &outPendingCompImageSetVersion,
&outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
@@ -1132,7 +1136,7 @@
responseMsg =
reinterpret_cast<const pldm_msg*>(invalidGetFwParamsResponse2.data());
rc = decode_get_firmware_parameters_resp(
- responseMsg, invalidGetFwParamsResponse2.size(), &outResp,
+ responseMsg, invalidGetFwParamsResponse2.size() - hdrSize, &outResp,
&outActiveCompImageSetVersion, &outPendingCompImageSetVersion,
&outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
@@ -1145,7 +1149,7 @@
responseMsg =
reinterpret_cast<const pldm_msg*>(invalidGetFwParamsResponse3.data());
rc = decode_get_firmware_parameters_resp(
- responseMsg, invalidGetFwParamsResponse3.size(), &outResp,
+ responseMsg, invalidGetFwParamsResponse3.size() - hdrSize, &outResp,
&outActiveCompImageSetVersion, &outPendingCompImageSetVersion,
&outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
@@ -1158,7 +1162,7 @@
responseMsg =
reinterpret_cast<const pldm_msg*>(invalidGetFwParamsResponse4.data());
rc = decode_get_firmware_parameters_resp(
- responseMsg, invalidGetFwParamsResponse4.size(), &outResp,
+ responseMsg, invalidGetFwParamsResponse4.size() - hdrSize, &outResp,
&outActiveCompImageSetVersion, &outPendingCompImageSetVersion,
&outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
@@ -1170,7 +1174,7 @@
responseMsg =
reinterpret_cast<const pldm_msg*>(invalidGetFwParamsResponse5.data());
rc = decode_get_firmware_parameters_resp(
- responseMsg, invalidGetFwParamsResponse5.size(), &outResp,
+ responseMsg, invalidGetFwParamsResponse5.size() - hdrSize, &outResp,
&outActiveCompImageSetVersion, &outPendingCompImageSetVersion,
&outCompParameterTable);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);