diff options
author | Patrick Steinhardt <ps@pks.im> | 2019-09-19 12:24:06 +0200 |
---|---|---|
committer | Patrick Steinhardt <ps@pks.im> | 2020-03-26 16:20:18 +0100 |
commit | 3c605da6e7a5aabde3d1929494347a8b8b3f4f51 (patch) | |
tree | 0d0bbfc6d96240c5ed9d0c85861d031295d3a6a5 | |
parent | 93dc8a04ec89bea1e8401e2fd7a9d9ef31e67537 (diff) | |
download | libgit2-3c605da6e7a5aabde3d1929494347a8b8b3f4f51.tar.gz |
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.
-rw-r--r-- | src/buffer.c | 3 | ||||
-rw-r--r-- | tests/core/buffer.c | 20 |
2 files changed, 22 insertions, 1 deletions
diff --git a/src/buffer.c b/src/buffer.c index dff2c37eb..61cf9675b 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -18,7 +18,8 @@ char git_buf__initbuf[1]; char git_buf__oom[1]; #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; diff --git a/tests/core/buffer.c b/tests/core/buffer.c index 4ef1274aa..3721ed8ab 100644 --- a/tests/core/buffer.c +++ b/tests/core/buffer.c @@ -1220,3 +1220,23 @@ void test_core_buffer__dont_hit_infinite_loop_when_resizing(void) 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")); +} |