summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlan Antonuk <alan.antonuk@gmail.com>2019-11-03 23:50:07 -0800
committerAlan Antonuk <alan.antonuk@gmail.com>2019-11-04 00:18:04 -0800
commitfc85be7123050b91b054e45b91c78d3241a5047a (patch)
tree749d9dac5af17f55e87948dc9840335f4a8c4866
parent60adf5f8817f32b25a99aa54ba186a29208c7942 (diff)
downloadrabbitmq-c-fc85be7123050b91b054e45b91c78d3241a5047a.tar.gz
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.
-rw-r--r--librabbitmq/amqp_connection.c15
1 files 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;
}