[PATCH 1/6] object-file: check size of inflated loose object against its header
- From
Elia Pinto <gitter.spiros@gmail.com>
- Date
- Oct 30, 2026, 10:28 UTC
- Message-ID
- <6c2efb87fbb95d3d24ac8e4ef34b586c2b25980e.1793355390.git.gitter.spiros@gmail.com>
- In-Reply-To
- <cover.1793355390.git.gitter.spiros@gmail.com>
unpack_loose_rest() allocates a buffer of the size given in the object header and inflates the rest of the stream into it, but it never checks that the stream actually produced that many bytes. If the compressed data ends early we return success with the tail of the buffer never written, and callers like odb_read_object() hand it out as object content. For example, "git cat-file commit" on a loose object whose header says "commit 200" but which contains only 44 bytes prints 156 bytes of uninitialized heap memory.
The opposite case slips through, too: when the whole object fits into the buffer filled by unpack_loose_header(), we copy only "size" bytes out of it, the stream has already ended, and the extra data is silently dropped.
The packfile reader in unpack_compressed_entry() and index-pack already compare total_out with the expected size. Do the same here and report the object as corrupt if they differ. Today fsck notices the first case only by accident, because it hashes the uninitialized bytes and gets a mismatch.
Teach loose_obj() in lib-loose.sh to write a bogus size, so that the tests can create such objects.
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> --- object-file.c | 15 +++++++++++---- t/lib-loose.sh | 5 ++++- t/t1060-object-corruption.sh | 28 ++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/object-file.c b/object-file.c index c049bf13f7..6d39138511 100644 --- a/object-file.c +++ b/object-file.c @@ -212,14 +212,14 @@ void *unpack_loose_rest(git_zstream *stream, void *buffer, unsigned long size, const struct object_id *oid) { - size_t bytes = strlen(buffer) + 1, n; + size_t hdrlen = strlen(buffer) + 1, bytes, n; unsigned char *buf = xmallocz(size); int status = Z_OK; - n = stream->total_out - bytes; + n = stream->total_out - hdrlen; if (n > size) n = size; - memcpy(buf, (char *) buffer + bytes, n); + memcpy(buf, (char *) buffer + hdrlen, n); bytes = n; if (bytes <= size) { /* @@ -244,7 +244,14 @@ void *unpack_loose_rest(git_zstream *stream, } } - if (status != Z_STREAM_END) { + /* + * Make sure that the stream produced exactly as many bytes as the + * header claims. Otherwise we would hand back a buffer whose tail + * was never written (if the stream is short), or silently drop data + * (if the stream is long but ended before we noticed). + */ + if (status != Z_STREAM_END || + stream->total_out - hdrlen != size) { error(_("corrupt loose object '%s'"), oid_to_hex(oid)); FREE_AND_NULL(buf); } else if (stream->avail_in) { diff --git a/t/lib-loose.sh b/t/lib-loose.sh index 3613631eaf..807fca6f05 100644 --- a/t/lib-loose.sh +++ b/t/lib-loose.sh @@ -5,9 +5,12 @@ # # oid=$(echo foo | loose_obj .git/objects blob) # +# If $3 is given, it is written as the size in the object header instead +# of the real size of the contents. +# loose_obj () { cat >tmp_loose.content && - size=$(wc -c <tmp_loose.content) && + size=${3:-$(wc -c <tmp_loose.content)} && { # Do not quote $size here; we want the shell # to strip whitespace that "wc" adds on some platforms. diff --git a/t/t1060-object-corruption.sh b/t/t1060-object-corruption.sh index d2ef468b45..bd3d1adf5d 100755 --- a/t/t1060-object-corruption.sh +++ b/t/t1060-object-corruption.sh @@ -3,6 +3,7 @@ test_description='see how we handle various forms of corruption' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-loose.sh # convert "1234abcd" to ".git/objects/12/34abcd" obj_to_file() { @@ -163,4 +164,31 @@ test_expect_success 'corrupted loose commit can be read from alternate' ' test_grep "inflate: data stream error" err ' +test_expect_success 'setup loose objects with bogus header size' ' + git init bogus-size && + ( + cd bogus-size && + printf "short" | + loose_obj .git/objects commit 200 >../oid-too-big && + printf "much longer" | + loose_obj .git/objects commit 3 >../oid-too-small + ) +' + +test_expect_success 'reading loose object shorter than its header fails' ' + test_must_fail git -C bogus-size cat-file commit $(cat oid-too-big) 2>err && + test_grep "corrupt loose object" err +' + +test_expect_success 'reading loose object longer than its header fails' ' + test_must_fail git -C bogus-size cat-file commit $(cat oid-too-small) 2>err && + test_grep "corrupt loose object" err +' + +test_expect_success 'fsck reports loose objects with bogus header size' ' + test_must_fail git -C bogus-size fsck 2>err && + test_grep "corrupt loose object .$(cat oid-too-big)" err && + test_grep "corrupt loose object .$(cat oid-too-small)" err +' + test_done
-- 2.53.0