[PATCH 0/6] loose object size checks and a few size truncations

[PATCH 0/6] loose object size checks and a few size truncations

[Elia Pinto]

These patches come from the same search for truncated sizes on LLP64
(64-bit Windows, where "long" is 32 bits but size_t is 64 bits) as the
"use size_t for a few more object and buffer sizes" series:

  https://lore.kernel.org/git/cover.1793352943.git.gitter.spiros@gmail.com/

Its cover letter explains the method (a MinGW cross build and
clang-tidy with -Wshorten-64-to-32). For these I followed each value
back to where it comes from and forward to where it is used, because
they can lead to out-of-bounds memory accesses. For that reason I sent
all of them but 3/6 to the git-security list first; they do not
consider them security issues, so here they are. While checking the
loose object reader I also found two missing size checks that have
nothing to do with LLP64 (1/6 and 3/6).

 1/6 A loose object whose zlib stream is shorter than the size in its
     header is accepted, and the uninitialized tail of the buffer is
     handed out as object content (all platforms; e.g. "git cat-file
     commit" prints heap memory). A longer stream is silently cut.

 2/6 On Windows the size from the loose object header is truncated, so
     the caller gets a buffer much smaller than the size it is told,
     and reads beyond its end.

 3/6 The same check as 1/6 for the streaming reader of loose objects,
     which today streams out a short or long object without an error.

 4/6 "git cat-file --batch" writes memory past the end of objects
     between 2GiB and 4GiB (all platforms; non-blob objects, or blobs
     with --filters/--textconv).

 5/6 On Windows, concatenating notes that add up to 4GiB or more
     overflows a heap buffer.

 6/6 On Windows, diffing a 2-4GiB file as text reads one byte before
     its buffer.

1/6 and 3/6 come with tests, which are cheap, and fail without the
fix. The others would need objects of several GiB (and 2/6, 5/6 and
6/6 a 32-bit "long"), so their commit messages describe how to
reproduce them instead. I have not run anything on Windows: the
Windows-only issues are based on reading the code and on the
cross-compiled build.

5/6 passes the combined size to odb_write_object() through
cast_size_t_to_ulong(), as the length parameter is still an "unsigned
long" on master. With 1/4 of the series above, which makes it a
size_t, the cast becomes unnecessary but harmless; the two series do
not conflict.

The series is based on master (b9e5cff180).

Elia Pinto (6):
  object-file: check size of inflated loose object against its header
  object-file: use size_t for loose object sizes
  odb/source-loose: check object size when streaming loose objects
  cat-file: use size_t for the length passed to batch_write()
  notes: avoid truncating the size of concatenated notes
  diff: use size_t in count_trailing_blank()

 builtin/cat-file.c           |  2 +-
 diff.c                       |  2 +-
 notes.c                      |  7 +++----
 object-file.c                | 29 ++++++++++++++++++-----------
 object-file.h                |  6 +++---
 odb/source-loose.c           | 22 ++++++++++++++++++----
 t/lib-loose.sh               |  5 ++++-
 t/t1006-cat-file.sh          | 18 ++++++++++++++++++
 t/t1060-object-corruption.sh | 28 ++++++++++++++++++++++++++++
 9 files changed, 94 insertions(+), 25 deletions(-)


base-commit: b9e5cff1803ab24582201ef059516eb16014b9d9
-- 
2.53.0

[PATCH 1/6] object-file: check size of inflated loose object against its header

[Elia Pinto]

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

[PATCH 2/6] object-file: use size_t for loose object sizes

[Elia Pinto]

Since c6a4629e32 (odb: use size_t for object_info.sizep and the size
APIs, 2026-06-15), parse_loose_header() reports the object size as a
size_t. But unpack_loose_rest() still takes an "unsigned long", which
is only 32 bits on Windows. A header claiming 4GiB + 5 bytes thus
makes us allocate a 6-byte buffer, the size check added by the previous
commit is happy because the stream really has 5 bytes, and the caller
gets that buffer back together with *oi->sizep set to 4GiB + 5. Any
user of the object then reads far beyond the end of the buffer;
read_loose_object() already does so when it hashes the contents.

Use size_t for the size in unpack_loose_rest() and check_stream_oid(),
and while at it for the size of the mapped file in map_fd(),
odb_source_loose_map_object() and unpack_loose_header(), so that the
whole loose object reading path agrees on the type.

There is no test, as the reader allocates a buffer of the size claimed
in the header before it can notice that the stream is short, so a test
would need 4GiB of memory. The problem can be reproduced on Windows
with the loose_obj() helper from the previous commit:

	printf short |
	loose_obj .git/objects commit $((4 * 1024 * 1024 * 1024 + 5))

followed by "git cat-file commit" on the resulting object ID.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 object-file.c      | 14 +++++++-------
 object-file.h      |  6 +++---
 odb/source-loose.c |  8 ++++----
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/object-file.c b/object-file.c
index 6d39138511..0fc240b1e8 100644
--- a/object-file.c
+++ b/object-file.c
@@ -155,7 +155,7 @@ int stream_object_signature(struct repository *r,
  * Map and close the given loose object fd. The path argument is used for
  * error reporting.
  */
-static void *map_fd(int fd, const char *path, unsigned long *size)
+static void *map_fd(int fd, const char *path, size_t *size)
 {
 	void *map = NULL;
 	struct stat st;
@@ -176,9 +176,9 @@ static void *map_fd(int fd, const char *path, unsigned long *size)
 
 enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
 						    unsigned char *map,
-						    unsigned long mapsize,
+						    size_t mapsize,
 						    void *buffer,
-						    unsigned long bufsiz)
+						    size_t bufsiz)
 {
 	int status;
 
@@ -209,7 +209,7 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
 }
 
 void *unpack_loose_rest(git_zstream *stream,
-			void *buffer, unsigned long size,
+			void *buffer, size_t size,
 			const struct object_id *oid)
 {
 	size_t hdrlen = strlen(buffer) + 1, bytes, n;
@@ -1135,7 +1135,7 @@ int for_each_loose_file_in_source(struct odb_source *source,
 
 static int check_stream_oid(git_zstream *stream,
 			    const char *hdr,
-			    unsigned long size,
+			    size_t size,
 			    const char *path,
 			    const struct object_id *expected_oid,
 			    const struct git_hash_algo *algop)
@@ -1143,7 +1143,7 @@ static int check_stream_oid(git_zstream *stream,
 	struct git_hash_ctx c;
 	struct object_id real_oid;
 	unsigned char buf[4096];
-	unsigned long total_read;
+	size_t total_read;
 	int status = Z_OK;
 
 	git_hash_init(&c, algop);
@@ -1203,7 +1203,7 @@ int read_loose_object(struct repository *repo,
 	int ret = -1;
 	int fd;
 	void *map = NULL;
-	unsigned long mapsize;
+	size_t mapsize;
 	git_zstream stream;
 	char hdr[MAX_HEADER_LEN];
 	size_t *size = oi->sizep;
diff --git a/object-file.h b/object-file.h
index f44758c4f8..a8da9dcd22 100644
--- a/object-file.h
+++ b/object-file.h
@@ -157,11 +157,11 @@ enum unpack_loose_header_result {
  */
 enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
 						    unsigned char *map,
-						    unsigned long mapsize,
+						    size_t mapsize,
 						    void *buffer,
-						    unsigned long bufsiz);
+						    size_t bufsiz);
 void *unpack_loose_rest(git_zstream *stream,
-			void *buffer, unsigned long size,
+			void *buffer, size_t size,
 			const struct object_id *oid);
 
 int parse_loose_header(const char *hdr, struct object_info *oi);
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 50cb388c35..d77e93e7c4 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -73,7 +73,7 @@ static int read_object_info_from_path(struct odb_source_loose *loose,
 {
 	int ret;
 	int fd;
-	unsigned long mapsize;
+	size_t mapsize;
 	void *map = NULL;
 	git_zstream stream, *stream_to_end = NULL;
 	char hdr[MAX_HEADER_LEN];
@@ -250,7 +250,7 @@ static int open_loose_object(struct odb_source_loose *loose,
 
 static void *odb_source_loose_map_object(struct odb_source_loose *loose,
 					 const struct object_id *oid,
-					 unsigned long *size)
+					 size_t *size)
 {
 	const char *p;
 	int fd = open_loose_object(loose, oid, &p);
@@ -285,7 +285,7 @@ struct odb_loose_read_stream {
 		ODB_LOOSE_READ_STREAM_ERROR,
 	} z_state;
 	void *mapped;
-	unsigned long mapsize;
+	size_t mapsize;
 	char hdr[32];
 	int hdr_avail;
 	int hdr_used;
@@ -356,7 +356,7 @@ static int odb_source_loose_read_object_stream(struct odb_stream **out,
 	struct odb_source_loose *loose = odb_source_loose_downcast(source);
 	struct object_info oi = OBJECT_INFO_INIT;
 	struct odb_loose_read_stream *st;
-	unsigned long mapsize;
+	size_t mapsize;
 	void *mapped;
 
 	mapped = odb_source_loose_map_object(loose, oid, &mapsize);
-- 
2.53.0

[PATCH 3/6] odb/source-loose: check object size when streaming loose objects

[Elia Pinto]

read_istream_loose() inflates a loose object until zlib reports the end
of the stream, but never compares the number of bytes it produced with
the size given in the object header. A corrupt object whose stream is
shorter than its header claims is thus streamed out without any error,
e.g. "git cat-file blob" exits with 0 after writing fewer bytes than
"git cat-file -s" reports. The same happens if the stream is longer than
the header claims.

Keep track of how many bytes we handed out and report the object as
corrupt if that does not match the size from the header once the stream
ends.

The tests create such objects with the loose_obj() helper, which can
already write a bogus size into the header.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 odb/source-loose.c  | 14 ++++++++++++++
 t/t1006-cat-file.sh | 18 ++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/odb/source-loose.c b/odb/source-loose.c
index d77e93e7c4..2233714ac4 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -289,6 +289,8 @@ struct odb_loose_read_stream {
 	char hdr[32];
 	int hdr_avail;
 	int hdr_used;
+	size_t total_read;
+	struct object_id oid;
 };
 
 static ssize_t read_istream_loose(struct odb_stream *_st, char *buf, size_t sz)
@@ -326,6 +328,16 @@ static ssize_t read_istream_loose(struct odb_stream *_st, char *buf, size_t sz)
 
 		if (status == Z_STREAM_END) {
 			git_inflate_end(&st->z);
+			/*
+			 * The object must be exactly as large as its
+			 * header claims.
+			 */
+			if (st->total_read + total_read != st->base.size) {
+				error(_("corrupt loose object '%s'"),
+				      oid_to_hex(&st->oid));
+				st->z_state = ODB_LOOSE_READ_STREAM_ERROR;
+				return -1;
+			}
 			st->z_state = ODB_LOOSE_READ_STREAM_DONE;
 			break;
 		}
@@ -335,6 +347,7 @@ static ssize_t read_istream_loose(struct odb_stream *_st, char *buf, size_t sz)
 			return -1;
 		}
 	}
+	st->total_read += total_read;
 	return total_read;
 }
 
@@ -388,6 +401,7 @@ static int odb_source_loose_read_object_stream(struct odb_stream **out,
 
 	st->mapped = mapped;
 	st->mapsize = mapsize;
+	oidcpy(&st->oid, oid);
 	st->hdr_used = strlen(st->hdr) + 1;
 	st->hdr_avail = st->z.total_out;
 	st->z_state = ODB_LOOSE_READ_STREAM_INUSE;
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 72fc8acc61..31a76c1b5d 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -1481,4 +1481,22 @@ test_objects_filter "object:type=commit"
 test_objects_filter "object:type=tag"
 test_objects_filter "object:type=tree"
 
+test_expect_success 'setup blobs with bogus size in header' '
+	git init bogus-size &&
+	echo_without_newline short |
+	loose_obj bogus-size/.git/objects blob 100 >oid-too-big &&
+	echo_without_newline "much longer" |
+	loose_obj bogus-size/.git/objects blob 3 >oid-too-small
+'
+
+test_expect_success 'streaming blob shorter than its header fails' '
+	test_must_fail git -C bogus-size cat-file blob $(cat oid-too-big) >out 2>err &&
+	test_grep "corrupt loose object" err
+'
+
+test_expect_success 'streaming blob longer than its header fails' '
+	test_must_fail git -C bogus-size cat-file blob $(cat oid-too-small) >out 2>err &&
+	test_grep "corrupt loose object" err
+'
+
 test_done
-- 
2.53.0

[PATCH 4/6] cat-file: use size_t for the length passed to batch_write()

[Elia Pinto]

batch_write() takes the length of the data as an "int", but its callers
pass the size of whole objects as returned by odb_read_object() or
filter_object(), which is a size_t. For an object between 2GiB and
4GiB the length wraps to a negative value, which fwrite() and
write_or_die() then see as an enormous size_t. We happily write the
object and then keep going past the end of the buffer, sending whatever
memory follows to stdout, until we hit an unmapped page and die with
"write error: Bad address". Objects of 4GiB and more are truncated
instead, which desynchronizes the --batch output.

This affects all platforms, not only those with a 32-bit "long". It can
be triggered with "--batch" on any non-blob object, or on blobs with
--filters or --textconv, as blobs are otherwise streamed. For example:

	blob=$(test-tool genzeros $((3 * 1024 * 1024 * 1024)) |
	       git hash-object -w --stdin) &&
	echo "$blob big" |
	git cat-file --batch="%(objectname) %(rest)" --filters >out

fails with "write error: Bad address"; with stdout going to /dev/null,
strace shows write() being called on addresses far beyond the end of the
3GiB buffer. There is no test, as it would need several GiB of memory
and disk.

Both fwrite() and write_or_die() take a size_t already, so just change
the parameter type.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/cat-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 8870a210ec..c846fe3116 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -425,7 +425,7 @@ static void expand_format(struct strbuf *sb, const char *start,
 	}
 }
 
-static void batch_write(struct batch_options *opt, const void *data, int len)
+static void batch_write(struct batch_options *opt, const void *data, size_t len)
 {
 	if (opt->buffer_output) {
 		if (fwrite(data, 1, len, stdout) != len)
-- 
2.53.0

[PATCH 5/6] notes: avoid truncating the size of concatenated notes

[Elia Pinto]

combine_notes_concatenate() reads both notes with odb_read_object(),
which returns their sizes as size_t, but computes the size of the
combined buffer in an "unsigned long". On platforms where that type is
32 bits wide, like Windows, two notes adding up to 4GiB or more give a
small buf_len: we allocate that many bytes and then memcpy() the full
contents of both notes into the buffer, writing far past its end.

The notes can come from a fetched notes ref. Besides notes merges with
the "union" strategy and notes.rewriteMode=concatenate (the default),
the function is also used when loading a notes tree that contains more
than one note for the same object, e.g. at different fanout levels, so
merely displaying such notes is enough to trigger it.

Compute the size as a size_t with st_add3(), and go through
cast_size_t_to_ulong() when passing it to odb_write_object(), whose
length parameter is still an "unsigned long". That makes us die on such
platforms instead of writing a truncated blob, until the write API
learns about size_t, too.

No test, as triggering this needs a platform with a 32-bit "long" and
notes larger than 4GiB.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 notes.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/notes.c b/notes.c
index ec9c2cb150..05da28b8d4 100644
--- a/notes.c
+++ b/notes.c
@@ -811,8 +811,7 @@ int combine_notes_concatenate(struct object_id *cur_oid,
 			      const struct object_id *new_oid)
 {
 	char *cur_msg = NULL, *new_msg = NULL, *buf;
-	unsigned long buf_len;
-	size_t cur_len, new_len;
+	size_t cur_len, new_len, buf_len;
 	enum object_type cur_type, new_type;
 	int ret;
 
@@ -839,7 +838,7 @@ int combine_notes_concatenate(struct object_id *cur_oid,
 		cur_len--;
 
 	/* concatenate cur_msg and new_msg into buf */
-	buf_len = cur_len + 2 + new_len;
+	buf_len = st_add3(cur_len, 2, new_len);
 	buf = (char *) xmalloc(buf_len);
 	memcpy(buf, cur_msg, cur_len);
 	buf[cur_len] = '\n';
@@ -850,7 +849,7 @@ int combine_notes_concatenate(struct object_id *cur_oid,
 
 	/* create a new blob object from buf */
 	ret = odb_write_object(the_repository->objects, buf,
-			       buf_len, OBJ_BLOB, cur_oid);
+			       cast_size_t_to_ulong(buf_len), OBJ_BLOB, cur_oid);
 	free(buf);
 	return ret;
 }
-- 
2.53.0

[PATCH 6/6] diff: use size_t in count_trailing_blank()

[Elia Pinto]

Since 3c7a31204f (xdiff: use size_t for buffer sizes, 2026-09-30),
mmfile_t stores its size as a size_t, but count_trailing_blank() still
copies it into a "long". On platforms where "long" is 32 bits wide,
like Windows, a buffer between 2GiB and 4GiB gives a negative size, and
"ptr += size - 1" points about 2GiB before the start of the buffer,
which we then dereference.

check_blank_at_eof() calls this function whenever the whitespace rule
includes blank-at-eof, which is the default, and it does so before
xdi_diff() rejects inputs larger than MAX_XDIFF_SIZE. Large files are
normally treated as binary and never get here, but a "diff" attribute
or a textconv driver makes us diff them as text. A repository can ship
such an attribute in its .gitattributes, so "git show" or "git log -p"
on a commit adding a 3GiB file can be made to crash.

Use size_t; the rest of the function works unchanged with it.

No test, as triggering this needs a platform with a 32-bit "long" and
a blob larger than 2GiB.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index b4ac17f8ef..e42e53d43e 100644
--- a/diff.c
+++ b/diff.c
@@ -711,7 +711,7 @@ static unsigned long diff_filespec_size(struct repository *r,
 static int count_trailing_blank(mmfile_t *mf)
 {
 	char *ptr = mf->ptr;
-	long size = mf->size;
+	size_t size = mf->size;
 	int cnt = 0;
 
 	if (!size)
-- 
2.53.0