fw-update: prevent firmware data request overflow RequestFirmwareData supplies a 32-bit offset and length. Their sum can wrap before the component bounds check, accept an out-of-range request, and index outside the selected component image. Validate the offset before subtracting it from the component size, then derive padding from the remaining bytes so no addition can overflow. Widen the package seek arithmetic and add regressions for a wrapped offset, exact baseline padding, and an offset beyond the component end. Change-Id: I4092729cfb9d9790f10489d27bed039ded5c0512 Signed-off-by: Ugur Ozer <info@airiskmanagement.ca>
diff --git a/fw-update/device_updater.cpp b/fw-update/device_updater.cpp index 39fbd2d..b1a4164 100644 --- a/fw-update/device_updater.cpp +++ b/fw-update/device_updater.cpp
@@ -609,7 +609,7 @@ return response; } - if (offset + length > compSize + PLDM_FWUP_BASELINE_TRANSFER_SIZE) + if (offset > compSize) { rc = encode_request_firmware_data_resp( request->hdr.instance_id, PLDM_FWUP_DATA_OUT_OF_RANGE, responseMsg, @@ -623,10 +623,24 @@ return response; } + const auto componentBytesRemaining = compSize - offset; size_t padBytes = 0; - if (offset + length > compSize) + if (length > componentBytesRemaining) { - padBytes = offset + length - compSize; + padBytes = length - componentBytesRemaining; + if (padBytes > PLDM_FWUP_BASELINE_TRANSFER_SIZE) + { + rc = encode_request_firmware_data_resp( + request->hdr.instance_id, PLDM_FWUP_DATA_OUT_OF_RANGE, + responseMsg, sizeof(completionCode)); + if (rc) + { + error( + "Failed to encode request firmware date response for endpoint ID '{EID}', response code '{RC}'", + "EID", eid, "RC", rc); + } + return response; + } } if (componentIndex < progress.size()) @@ -642,7 +656,7 @@ response.resize(sizeof(pldm_msg_hdr) + sizeof(completionCode) + length); responseMsg = new (response.data()) pldm_msg; - package.seekg(compOffset + offset); + package.seekg(static_cast<std::streamoff>(compOffset) + offset); package.read( reinterpret_cast<char*>( response.data() + sizeof(pldm_msg_hdr) + sizeof(completionCode)),
diff --git a/fw-update/test/device_updater_test.cpp b/fw-update/test/device_updater_test.cpp index 6dcc954..66bdda3 100644 --- a/fw-update/test/device_updater_test.cpp +++ b/fw-update/test/device_updater_test.cpp
@@ -136,6 +136,62 @@ EXPECT_EQ(deviceUpdater.getProgress(), 48); } +TEST_F(DeviceUpdaterTest, RejectWrappedOffsetBeforeComponent) +{ + DeviceUpdater deviceUpdater(0, package, fwDeviceIDRecord, compImageInfos, + compInfo, 512, nullptr); + + // The component starts at byte 139. In the affected implementation, + // compOffset + offset wraps to zero and offset + length wraps to 373. + constexpr std::array<uint8_t, sizeof(pldm_msg_hdr) + + sizeof(pldm_request_firmware_data_req)> + reqFwDataReq{0x8A, 0x05, 0x15, 0x75, 0xFF, 0xFF, + 0xFF, 0x00, 0x02, 0x00, 0x00}; + auto requestMsg = reinterpret_cast<const pldm_msg*>(reqFwDataReq.data()); + auto response = deviceUpdater.requestFwData( + requestMsg, sizeof(pldm_request_firmware_data_req)); + + EXPECT_EQ(response.size(), sizeof(pldm_msg_hdr) + sizeof(uint8_t)); + EXPECT_EQ(response[sizeof(pldm_msg_hdr)], PLDM_FWUP_DATA_OUT_OF_RANGE); +} + +TEST_F(DeviceUpdaterTest, AllowBaselinePaddingAtComponentEnd) +{ + DeviceUpdater deviceUpdater(0, package, fwDeviceIDRecord, compImageInfos, + compInfo, 512, nullptr); + + constexpr std::array<uint8_t, sizeof(pldm_msg_hdr) + + sizeof(pldm_request_firmware_data_req)> + reqFwDataReq{0x8A, 0x05, 0x15, 0x00, 0x04, 0x00, + 0x00, 0x20, 0x00, 0x00, 0x00}; + auto requestMsg = reinterpret_cast<const pldm_msg*>(reqFwDataReq.data()); + auto response = deviceUpdater.requestFwData( + requestMsg, sizeof(pldm_request_firmware_data_req)); + + EXPECT_EQ(response.size(), sizeof(pldm_msg_hdr) + sizeof(uint8_t) + 32); + EXPECT_EQ(response[sizeof(pldm_msg_hdr)], PLDM_SUCCESS); + EXPECT_TRUE( + std::all_of(response.begin() + sizeof(pldm_msg_hdr) + sizeof(uint8_t), + response.end(), [](uint8_t value) { return value == 0; })); +} + +TEST_F(DeviceUpdaterTest, RejectOffsetPastComponentEnd) +{ + DeviceUpdater deviceUpdater(0, package, fwDeviceIDRecord, compImageInfos, + compInfo, 512, nullptr); + + constexpr std::array<uint8_t, sizeof(pldm_msg_hdr) + + sizeof(pldm_request_firmware_data_req)> + reqFwDataReq{0x8A, 0x05, 0x15, 0x01, 0x04, 0x00, + 0x00, 0x20, 0x00, 0x00, 0x00}; + auto requestMsg = reinterpret_cast<const pldm_msg*>(reqFwDataReq.data()); + auto response = deviceUpdater.requestFwData( + requestMsg, sizeof(pldm_request_firmware_data_req)); + + EXPECT_EQ(response.size(), sizeof(pldm_msg_hdr) + sizeof(uint8_t)); + EXPECT_EQ(response[sizeof(pldm_msg_hdr)], PLDM_FWUP_DATA_OUT_OF_RANGE); +} + TEST_F(DeviceUpdaterTest, FullUpdateProgress) { DeviceUpdater deviceUpdater(0, package, fwDeviceIDRecord, compImageInfos,