From 62e6ef2564885dd950725c35e6d831baf1ea8332 Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Mon, 13 May 2024 13:58:42 -0400 Subject: [PATCH] Check length of buffer (#106) * Check buffer length in decode Test plan: Added test that triggers stack-buffer-overflow in AddressSanitizer. * Update libamqpprox/amqpprox_method.cpp Co-authored-by: Jessica Winer --------- Co-authored-by: Jessica Winer --- libamqpprox/amqpprox_method.cpp | 5 +++++ tests/amqpprox_packetprocessor.t.cpp | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/libamqpprox/amqpprox_method.cpp b/libamqpprox/amqpprox_method.cpp index aa3f7c9..b535857 100644 --- a/libamqpprox/amqpprox_method.cpp +++ b/libamqpprox/amqpprox_method.cpp @@ -21,6 +21,11 @@ namespace amqpprox { bool Method::decode(Method *method, const void *buffer, std::size_t bufferLen) { const uint8_t *buf = static_cast(buffer); + + if (bufferLen < sizeof(method->classType) + sizeof(method->methodType)) { + return false; + } + memcpy(&method->classType, buf, sizeof(method->classType)); memcpy(&method->methodType, buf + sizeof(method->classType), diff --git a/tests/amqpprox_packetprocessor.t.cpp b/tests/amqpprox_packetprocessor.t.cpp index 9001402..e304432 100644 --- a/tests/amqpprox_packetprocessor.t.cpp +++ b/tests/amqpprox_packetprocessor.t.cpp @@ -83,6 +83,30 @@ TEST_F(PacketProcessorTest, HeaderPassedInOne) EXPECT_THAT(d_connector.state(), Eq(Connector::State::START_SENT)); } +TEST_F(PacketProcessorTest, MalformedStartOk) +{ + PacketProcessor processor(d_sessionState, d_connector); + + // Header + { + Buffer buffer((const void *)Constants::protocolHeader(), + Constants::protocolHeaderLength()); + buffer.seek(Constants::protocolHeaderLength()); + + processor.process(FlowType::INGRESS, buffer); + } + + { + const uint8_t data[] = { + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xce}; + size_t size = sizeof(data) / sizeof(*data); + Buffer buffer(data, size); + buffer.seek(size); + + processor.process(FlowType::INGRESS, buffer); + } +} + TEST_F(PacketProcessorTest, IncompleteHeader) { // If we give the PacketProcessor less than the full header, we shouldn't