summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <ps@pks.im>2019-06-21 15:53:54 +0200
committerEdward Thomson <ethomson@edwardthomson.com>2019-08-13 17:56:06 +0100
commit57a9ccd5e21f8b98885e392f193ee6a7ead79172 (patch)
tree369bc6022eb2315c12e353ab7dbabf010f6916fb
parentcb1439c9d32c059ee93216637a6d155306f76ab3 (diff)
downloadlibgit2-57a9ccd5e21f8b98885e392f193ee6a7ead79172.tar.gz
commit_list: fix possible buffer overflow in `commit_quick_parse`
The function `commit_quick_parse` provides a way to quickly parse parts of a commit without storing or verifying most of its metadata. The first thing it does is calculating the number of parents by skipping "parent " lines until it finds the first non-parent line. Afterwards, this parent count is passed to `alloc_parents`, which will allocate an array to store all the parent. To calculate the amount of storage required for the parents array, `alloc_parents` simply multiplicates the number of parents with the respective elements's size. This already screams "buffer overflow", and in fact this problem is getting worse by the result being cast to an `uint32_t`. In fact, triggering this is possible: git-hash-object(1) will happily write a commit with multiple millions of parents for you. I've stopped at 67,108,864 parents as git-hash-object(1) unfortunately soaks up the complete object without streaming anything to disk and thus will cause an OOM situation at a later point. The point here is: this commit was about 4.1GB of size but compressed down to 24MB and thus easy to distribute. The above doesn't yet trigger the buffer overflow, thus. As the array's elements are all pointers which are 8 bytes on 64 bit, we need a total of 536,870,912 parents to trigger the overflow to `0`. The effect is that we're now underallocating the array and do an out-of-bound writes. As the buffer is kindly provided by the adversary, this may easily result in code execution. Extrapolating from the test file with 67m commits to the one with 536m commits results in a factor of 8. Thus the uncompressed contents would be about 32GB in size and the compressed ones 192MB. While still easily distributable via the network, only servers will have that amount of RAM and not cause an out-of-memory condition previous to triggering the overflow. This at least makes this attack not an easy vector for client-side use of libgit2.
-rw-r--r--src/commit_list.c8
1 files changed, 6 insertions, 2 deletions
diff --git a/src/commit_list.c b/src/commit_list.c
index 44673d9bc..78b1f055c 100644
--- a/src/commit_list.c
+++ b/src/commit_list.c
@@ -69,11 +69,15 @@ static int commit_error(git_commit_list_node *commit, const char *msg)
static git_commit_list_node **alloc_parents(
git_revwalk *walk, git_commit_list_node *commit, size_t n_parents)
{
+ size_t bytes;
+
if (n_parents <= PARENTS_PER_COMMIT)
return (git_commit_list_node **)((char *)commit + sizeof(git_commit_list_node));
- return (git_commit_list_node **)git_pool_malloc(
- &walk->commit_pool, (n_parents * sizeof(git_commit_list_node *)));
+ if (git__multiply_sizet_overflow(&bytes, n_parents, sizeof(git_commit_list_node *)))
+ return NULL;
+
+ return (git_commit_list_node **)git_pool_malloc(&walk->commit_pool, bytes);
}