From fc85be7123050b91b054e45b91c78d3241a5047a Mon Sep 17 00:00:00 2001 From: Alan Antonuk Date: Sun, 3 Nov 2019 23:50:07 -0800 Subject: lib: check frame_size is >= INT32_MAX When parsing a frame header, validate that the frame_size is less than or equal to INT32_MAX. Given frame_max is limited between 0 and INT32_MAX in amqp_login and friends, this does not change the API. This prevents a potential buffer overflow when a malicious client sends a frame_size that is close to UINT32_MAX, in which causes an overflow when computing state->target_size resulting in a small value there. A buffer is then allocated with the small amount, then memcopy copies the frame_size writing to memory beyond the end of the buffer. --- librabbitmq/amqp_connection.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/librabbitmq/amqp_connection.c b/librabbitmq/amqp_connection.c index 034b2e9..b106f70 100644 --- a/librabbitmq/amqp_connection.c +++ b/librabbitmq/amqp_connection.c @@ -287,12 +287,21 @@ int amqp_handle_input(amqp_connection_state_t state, amqp_bytes_t received_data, case CONNECTION_STATE_HEADER: { amqp_channel_t channel; amqp_pool_t *channel_pool; - /* frame length is 3 bytes in */ + uint32_t frame_size; + channel = amqp_d16(amqp_offset(raw_frame, 1)); - state->target_size = - amqp_d32(amqp_offset(raw_frame, 3)) + HEADER_SIZE + FOOTER_SIZE; + /* frame length is 3 bytes in */ + frame_size = amqp_d32(amqp_offset(raw_frame, 3)); + /* To prevent the target_size calculation below from overflowing, check + * that the stated frame_size is smaller than a signed 32-bit. Given + * the library only allows configuring frame_max as an int32_t, and + * frame_size is uint32_t, the math below is safe from overflow. */ + if (frame_size >= INT32_MAX) { + return AMQP_STATUS_BAD_AMQP_DATA; + } + state->target_size = frame_size + HEADER_SIZE + FOOTER_SIZE; if ((size_t)state->frame_max < state->target_size) { return AMQP_STATUS_BAD_AMQP_DATA; } -- cgit v1.2.1