Commit 174b7a32 by Patrick Steinhardt

buffer: fix printing into out-of-memory buffer

Before printing into a `git_buf` structure, we always call `ENSURE_SIZE`
first. This macro will reallocate the buffer as-needed depending on
whether the current amount of allocated bytes is sufficient or not. If
`asize` is big enough, then it will just do nothing, otherwise it will
call out to `git_buf_try_grow`. But in fact, it is insufficient to only
check `asize`.

When we fail to allocate any more bytes e.g. via `git_buf_try_grow`,
then we set the buffer's pointer to `git_buf__oom`. Note that we touch
neither `asize` nor `size`. So if we just check `asize > targetsize`,
then we will happily let the caller of `ENSURE_SIZE` proceed with an
out-of-memory buffer. As a result, we will print all bytes into the
out-of-memory buffer instead, resulting in an out-of-bounds write.

Fix the issue by having `ENSURE_SIZE` verify that the buffer is not
marked as OOM. Add a test to verify that we're not writing into the OOM
buffer.
parent 208f1d7a
...@@ -18,7 +18,8 @@ char git_buf__initbuf[1]; ...@@ -18,7 +18,8 @@ char git_buf__initbuf[1];
char git_buf__oom[1]; char git_buf__oom[1];
#define ENSURE_SIZE(b, d) \ #define ENSURE_SIZE(b, d) \
if ((d) > (b)->asize && git_buf_grow((b), (d)) < 0)\ if ((b)->ptr == git_buf__oom || \
((d) > (b)->asize && git_buf_grow((b), (d)) < 0))\
return -1; return -1;
......
...@@ -1220,3 +1220,23 @@ void test_core_buffer__dont_hit_infinite_loop_when_resizing(void) ...@@ -1220,3 +1220,23 @@ void test_core_buffer__dont_hit_infinite_loop_when_resizing(void)
git_buf_dispose(&buf); git_buf_dispose(&buf);
} }
void test_core_buffer__avoid_printing_into_oom_buffer(void)
{
git_buf buf = GIT_BUF_INIT;
/* Emulate OOM situation with a previous allocation */
buf.asize = 8;
buf.ptr = git_buf__oom;
/*
* Print the same string again. As the buffer still has
* an `asize` of 8 due to the previous print,
* `ENSURE_SIZE` would not try to reallocate the array at
* all. As it didn't explicitly check for `git_buf__oom`
* in earlier versions, this would've resulted in it
* returning successfully and thus `git_buf_puts` would
* just print into the `git_buf__oom` array.
*/
cl_git_fail(git_buf_puts(&buf, "foobar"));
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment