Skip to content

Commit

Permalink
Check length of buffer (#106)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>

---------

Co-authored-by: Jessica Winer <[email protected]>
  • Loading branch information
ccotter and Jessica Winer authored May 13, 2024
1 parent d0faeca commit 62e6ef2
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
5 changes: 5 additions & 0 deletions libamqpprox/amqpprox_method.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ namespace amqpprox {
bool Method::decode(Method *method, const void *buffer, std::size_t bufferLen)
{
const uint8_t *buf = static_cast<const uint8_t *>(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),
Expand Down
24 changes: 24 additions & 0 deletions tests/amqpprox_packetprocessor.t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 62e6ef2

Please sign in to comment.