rde_handler: Check operationInitRequest over-read
It seems the handler had vulnerabilities to buffer over-reads found by
the fuzzer. Fix them and add unit test coverage.
Tested: Unit test
Signed-off-by: Brandon Kim <brandonkim@google.com>
Change-Id: I655c28c799f0eda072636d55bfef83727afc5df7
diff --git a/src/rde/rde_handler.cpp b/src/rde/rde_handler.cpp
index 6f6acfb..74f6204 100644
--- a/src/rde/rde_handler.cpp
+++ b/src/rde/rde_handler.cpp
@@ -50,14 +50,39 @@
RdeDecodeStatus RdeCommandHandler::operationInitRequest(
std::span<const uint8_t> rdeCommand)
{
+ // Ensure rdeCommand is large enough for the header.
+ if (rdeCommand.size() < sizeof(RdeOperationInitReqHeader))
+ {
+ stdplus::print(
+ stderr,
+ "RDE OperationInitRequest command is smaller than the expected header size. Received: {}, Expected: {}\n",
+ rdeCommand.size(), sizeof(RdeOperationInitReqHeader));
+ return RdeDecodeStatus::RdeInvalidCommand;
+ }
+
const RdeOperationInitReqHeader* header =
reinterpret_cast<const RdeOperationInitReqHeader*>(rdeCommand.data());
+
// Check if there is a payload. If not, we are not doing anything.
if (!header->containsRequestPayload)
{
return RdeDecodeStatus::RdeOk;
}
+ // Ensure rdeCommand is large enough for header + locator + declared
+ // payload.
+ size_t expectedTotalSize =
+ sizeof(RdeOperationInitReqHeader) + header->operationLocatorLength +
+ header->requestPayloadLength;
+ if (rdeCommand.size() < expectedTotalSize)
+ {
+ stdplus::print(
+ stderr,
+ "RDE OperationInitRequest command size is smaller than header + locator + declared payload size. Received: {}, Expected: {}\n",
+ rdeCommand.size(), expectedTotalSize);
+ return RdeDecodeStatus::RdeInvalidCommand;
+ }
+
if (header->operationType !=
static_cast<uint8_t>(RdeOperationInitType::RdeOpInitOperationUpdate))
{
diff --git a/test/rde_handler_test.cpp b/test/rde_handler_test.cpp
index 9240102..7977d69 100644
--- a/test/rde_handler_test.cpp
+++ b/test/rde_handler_test.cpp
@@ -121,6 +121,32 @@
EXPECT_EQ(status, RdeDecodeStatus::RdeOk);
}
+TEST_F(RdeCommandHandlerTest, OperationInitRequest_CmdTooSmallForHeader)
+{
+ std::vector<uint8_t> cmdData = {0x01, 0x02}; // Smaller than header
+ auto status = handler->decodeRdeCommand(
+ cmdData, RdeCommandType::RdeOperationInitRequest);
+ EXPECT_EQ(status, RdeDecodeStatus::RdeInvalidCommand);
+}
+
+TEST_F(RdeCommandHandlerTest,
+ OperationInitRequest_CmdTooSmallForDeclaredPayload)
+{
+ // Header declares payload, but actual data is too short.
+ // Header: containsPayload=true, opLocatorLength=1, requestPayloadLength=5
+ // Actual payload data provided: only 1 byte for locator, 0 for payload.
+ RdeOperationInitReqHeader header{};
+ header.containsRequestPayload = true;
+ header.operationLocatorLength = 1;
+ header.requestPayloadLength = 5; // Expects 5 bytes of payload
+ std::vector<uint8_t> cmdData(sizeof(header));
+ memcpy(cmdData.data(), &header, sizeof(header));
+ cmdData.push_back(0xAA); // Only 1 byte for locator, payload missing
+ auto status = handler->decodeRdeCommand(
+ cmdData, RdeCommandType::RdeOperationInitRequest);
+ EXPECT_EQ(status, RdeDecodeStatus::RdeInvalidCommand);
+}
+
TEST_F(RdeCommandHandlerTest, OperationInitRequest_UnsupportedOperationType)
{
auto cmd = createOpInitReqCmd(true, 0xFE, 0, 1, 0, 5, {1, 2, 3, 4, 5});