- revparse: Remove error-prone, redundant test
Related: RHEL-9503 - commit_list: fix possible buffer overflow in `commit_quick_parse` Resolves: RHEL-9503
This commit is contained in:
Brian C. Lane
parent
7f98900773
commit
95c5c804ce
@ -0,0 +1,74 @@
|
||||
From c31dcbfd93d85a008e95b23c129c7e8887f1316e Mon Sep 17 00:00:00 2001
|
||||
From: Patrick Steinhardt <ps@pks.im>
|
||||
Date: Fri, 21 Jun 2019 15:53:54 +0200
|
||||
Subject: [PATCH 6/9] 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.
|
||||
|
||||
(cherry picked from commit 3316f666566f768eb8aa8de521a5262524dc3424)
|
||||
---
|
||||
src/commit_list.c | 8 ++++++--
|
||||
1 file changed, 6 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/src/commit_list.c b/src/commit_list.c
|
||||
index 7df79bfd6..14d1c9813 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, (uint32_t)(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);
|
||||
}
|
||||
|
||||
|
||||
--
|
||||
2.43.0
|
||||
|
||||
34
0007-revparse-Remove-error-prone-redundant-test.patch
Normal file
34
0007-revparse-Remove-error-prone-redundant-test.patch
Normal file
@ -0,0 +1,34 @@
|
||||
From 9a3568814bd1d7a3e6522e0f61168d63c29efe97 Mon Sep 17 00:00:00 2001
|
||||
From: Carl Dong <accounts@carldong.me>
|
||||
Date: Mon, 9 May 2022 12:09:08 -0400
|
||||
Subject: [PATCH] revparse: Remove error-prone, redundant test
|
||||
|
||||
Originally introduced in: 776a6a8e5f8e258d31aded73c0ce38df6ac7bdc4
|
||||
|
||||
This test case has recently been fixed in bdab22384cc61d315005a65456a9f9563bb27c8f, but that fix will only last for a year. Next year the same problem will crop up and the test will need to be re-edited.
|
||||
|
||||
This is not ideal as
|
||||
- This test case becomes an unnecessary burden for developers
|
||||
- Downstream distros or even just users who want to build older versions of libgit2 are guaranteed to have this test fail
|
||||
|
||||
Furthermore, this test case is entirely unnecessary, as the functionality that was originally (see 776a6a8e5f8e258d31aded73c0ce38df6ac7bdc4) intended to be tested is well-covered by subsequent tests which specify a date instead of a "x ago" specification.
|
||||
---
|
||||
tests/refs/revparse.c | 2 --
|
||||
1 file changed, 2 deletions(-)
|
||||
|
||||
diff --git a/tests/refs/revparse.c b/tests/refs/revparse.c
|
||||
index 459188cf7..2bb19ff69 100644
|
||||
--- a/tests/refs/revparse.c
|
||||
+++ b/tests/refs/revparse.c
|
||||
@@ -400,8 +400,6 @@ void test_refs_revparse__date(void)
|
||||
* a65fedf HEAD@{1335806603 -0900}: commit:
|
||||
* be3563a HEAD@{1335806563 -0700}: clone: from /Users/ben/src/libgit2/tests/resour
|
||||
*/
|
||||
- test_object("HEAD@{10 years ago}", NULL);
|
||||
-
|
||||
test_object("HEAD@{1 second}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
|
||||
test_object("HEAD@{1 second ago}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
|
||||
test_object("HEAD@{2 days ago}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
|
||||
--
|
||||
2.43.0
|
||||
|
||||
10
libgit2.spec
10
libgit2.spec
@ -1,6 +1,6 @@
|
||||
Name: libgit2
|
||||
Version: 0.26.8
|
||||
Release: 2%{?dist}
|
||||
Release: 3%{?dist}
|
||||
Summary: C implementation of the Git core methods as a library with a solid API
|
||||
License: GPLv2 with exceptions
|
||||
URL: http://libgit2.github.com/
|
||||
@ -12,6 +12,8 @@ Patch0002: 0002-CMakeLists-increase-strict-aliasing-level-to-3.patch
|
||||
Patch0003: 0003-Disable-online-tests.patch
|
||||
Patch0004: 0004-tests-Increase-TOOBIG-value-used-on-i686.patch
|
||||
Patch0005: 0005-openssl-Use-the-system-profile-ciphers.patch
|
||||
Patch0006: 0006-commit_list-fix-possible-buffer-overflow-in-commit_q.patch
|
||||
Patch0007: 0007-revparse-Remove-error-prone-redundant-test.patch
|
||||
|
||||
BuildRequires: gcc
|
||||
BuildRequires: cmake
|
||||
@ -80,6 +82,12 @@ popd
|
||||
%{_includedir}/git2/
|
||||
|
||||
%changelog
|
||||
* Wed Feb 07 2024 Brian C. Lane <bcl@redhat.com> - 0.26.8-3
|
||||
- revparse: Remove error-prone, redundant test
|
||||
Related: RHEL-9503
|
||||
- commit_list: fix possible buffer overflow in `commit_quick_parse`
|
||||
Resolves: RHEL-9503
|
||||
|
||||
* Tue Jun 02 2020 Brian C. Lane <bcl@redhat.com> - 0.26.8-2
|
||||
- openssl: Use the system profile ciphers
|
||||
Resolves: rhbz#1842814
|
||||
|
||||
Loading…
Reference in New Issue
Block a user