Verify packet size before accessing checksum
Calculating checksum without verifying that the dataLengthBytes
is correct, could lead to potential security issues. This patch
fixes the issue.
Tested:
Unit tested
Change-Id: I2fa0deea99db7426924f7756a5dcd07e5e49121b
Signed-off-by: Kasun Athukorala <kasunath@google.com>
diff --git a/src/rde/rde_handler.cpp b/src/rde/rde_handler.cpp
index 2bd377a..1752ad9 100644
--- a/src/rde/rde_handler.cpp
+++ b/src/rde/rde_handler.cpp
@@ -241,6 +241,19 @@
const MultipartReceiveResHeader* header =
reinterpret_cast<const MultipartReceiveResHeader*>(
multiReceiveRespCmd.data());
+
+ // Validate that the total message size (header + data + checksum) does not
+ // exceed the actual size of the received buffer.
+ size_t expectedSize = sizeof(MultipartReceiveResHeader) +
+ header->dataLengthBytes + sizeof(uint32_t);
+ if (expectedSize != multiReceiveRespCmd.size())
+ {
+ stdplus::print(
+ stderr,
+ "Corruption detected: Invalid dataLengthBytes in header or not enough bytes for checksum.\n");
+ return RdeDecodeStatus::RdeInvalidCommand;
+ }
+
const uint8_t* checksumPtr =
multiReceiveRespCmd.data() + sizeof(MultipartReceiveResHeader) +
header->dataLengthBytes;
diff --git a/test/rde_handler_test.cpp b/test/rde_handler_test.cpp
index efc3d31..c13feaa 100644
--- a/test/rde_handler_test.cpp
+++ b/test/rde_handler_test.cpp
@@ -571,6 +571,28 @@
2); // Both dictionaries should now be valid
}
+TEST_F(RdeCommandHandlerTest, MultiPartReceiveResp_HandleCrc_MismatchedSize)
+{
+ // Header will claim 10 bytes of data.
+ MultipartReceiveResHeader header{};
+ header.transferFlag = static_cast<uint8_t>(
+ RdeMultiReceiveTransferFlag::RdeMRecFlagStartAndEnd);
+ header.nextDataTransferHandle = 1; // dummy resource ID
+ header.dataLengthBytes = 10;
+
+ // Create a command that is exactly the size of the header + data, which
+ // means it is missing the 4-byte checksum. This will pass the initial size
+ // check but fail the one in handleCrc.
+ size_t actualSize =
+ sizeof(MultipartReceiveResHeader) + header.dataLengthBytes;
+ std::vector<uint8_t> command(actualSize);
+ memcpy(command.data(), &header, sizeof(header));
+
+ auto status = handler->decodeRdeCommand(
+ command, RdeCommandType::RdeMultiPartReceiveResponse);
+ EXPECT_EQ(status, RdeDecodeStatus::RdeInvalidCommand);
+}
+
/**
* @brief Dummy values for annotation dictionary. We do not need the annotation
* dictionary. So this contains a dictionary with some dummy values. But the RDE