Fix unsigned underflow in buffer_check_limits() bounds check#273
Fix unsigned underflow in buffer_check_limits() bounds check#273kodareef5 wants to merge 1 commit intodovecot:mainfrom
Conversation
|
Thank you for your pull request, we'll take a look. |
| size_t new_size; | ||
|
|
||
| if (unlikely(buf->max_size - pos < data_size)) | ||
| if (unlikely(pos > buf->max_size || buf->max_size - pos < data_size)) |
There was a problem hiding this comment.
This is a good fix, although commit message could say that this doesn't affect buffers created with buffer_create_dynamic(), which are most buffers, since then max_size == SIZE_MAX. Also commit title needs a lib: prefix.
There was a problem hiding this comment.
Done. Added lib: prefix and noted the buffer_create_dynamic() / SIZE_MAX scope in the commit message.
src/lib/buffer.c
Outdated
| it's going to be filled with the newly appended data. */ | ||
| #ifndef DEBUG_FAST | ||
| if (buf->writable_size - buf->used < data_size) | ||
| if (buf->used > buf->writable_size || |
There was a problem hiding this comment.
This is unnecessary check, because used cannot be larger than writable_size. I guess an assert could be added here to be sure.
There was a problem hiding this comment.
Good point. Replaced the conditional guard with i_assert(buf->used <= buf->writable_size) instead.
When pos > buf->max_size, the unsigned subtraction buf->max_size - pos wraps to a large value instead of triggering the panic. Add an explicit check before the subtraction so out-of-range positions are caught. This does not affect buffers created with buffer_create_dynamic(), which use max_size == SIZE_MAX. Also add an i_assert in buffer_check_append_limits() to verify the invariant that used <= writable_size.
kodareef5
force-pushed
the
fix-buffer-check-underflow
branch
from
4737b87 to
47e2e0b
Compare
3 participants
When
pos > buf->max_size, the unsigned subtractionbuf->max_size - poswraps to a large value instead of triggering the panic. This causes the bounds check to pass when it should fail.Add
pos > buf->max_sizebefore the subtraction so the check correctly catches out-of-range positions.