[PATCH 0/4] use size_t when parsing and checking objects

[PATCH 0/4] use size_t when parsing and checking objects

[Elia Pinto]

This series continues the search for truncated sizes on platforms where
"long" is 32 bits (Windows). It covers the functions that parse, verify,
and check whole objects in memory: they take the size as an "unsigned
long", while their callers have a size_t, so above 4GiB the size gets
truncated.

Each patch addresses a specific component:
 - 1/4: a valid object reported as corrupt;
 - 2/4: fsck looks at only a prefix (e.g., .gitmodules);
 - 3/4: parsing of a prefix (with set_commit_buffer() now dying instead
   of caching a truncated size, as repo_get_commit_buffer() already
   does);
 - 4/4: "git tag -v" doesn't find the signature, plus the int index in
   copy_subject() on all platforms.

There are no out-of-bounds accesses; large objects only fail or are
handled as if they were shorter.

No tests are added because they would need objects over 4GiB in memory,
and they'd only detect the bug where long is 32 bits. I am happy to add
EXPENSIVE tests guarded by !LONG_IS_64BIT (like t1051) if reviewers
prefer.

Not covered here: the pack paths (index-pack, unpack-objects, pack
verification) still pass an "unsigned long" to fsck_object(); that's the
next series. The tree API (parse_tree_buffer(), init_tree_desc*()) is in
the tree-walk series:
  https://lore.kernel.org/git/cover.1793357680.git.gitter.spiros@gmail.com/
This series doesn't depend on the other series in flight and merges
cleanly with them. Based on master 50fe5cc63f.

Elia Pinto (4):
  object-file: use size_t in check_object_signature()
  fsck: use size_t for the size of objects to check
  object, commit, tag: use size_t when parsing object buffers
  tag, fmt-merge-msg, ref-filter: use size_t for message buffers

 commit.c              |  8 ++++----
 commit.h              |  6 +++---
 fmt-merge-msg.c       |  2 +-
 fsck.c                | 18 +++++++++---------
 fsck.h                |  6 +++---
 object-file-convert.c |  2 +-
 object-file.c         |  2 +-
 object-file.h         |  2 +-
 object.c              |  2 +-
 object.h              |  2 +-
 ref-filter.c          |  6 +++---
 tag.c                 |  4 ++--
 tag.h                 |  2 +-
 13 files changed, 31 insertions(+), 31 deletions(-)


base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7
-- 
2.53.0

[PATCH 1/4] object-file: use size_t in check_object_signature()

[Elia Pinto]

check_object_signature() takes the object size as an "unsigned long",
while its callers pass a size_t from odb_read_object(), and the internal
function it calls, hash_object_file(), already accepts a size_t. So the
size is narrowed only here, and specifically on Windows where "long"
is 32 bits wide.

On platforms with 32-bit longs, hashing an object of 4GiB or more
truncates the size, causing it to hash only a prefix. This mismatch
results in valid objects being incorrectly flagged as corrupt by
callers like parse_object() or git fsck.

Use size_t for the size parameter, matching what callers
already provide.

Triggering this requires processing 4GiB or more of input on a platform
where "long" is 32 bits, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 object-file.c | 2 +-
 object-file.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/object-file.c b/object-file.c
index c049bf13f7..3cb8f3fac1 100644
--- a/object-file.c
+++ b/object-file.c
@@ -111,7 +111,7 @@ int format_object_header(char *str, size_t size, enum object_type type,
 }
 
 int check_object_signature(struct repository *r, const struct object_id *oid,
-			   void *buf, unsigned long size,
+			   void *buf, size_t size,
 			   enum object_type type)
 {
 	const struct git_hash_algo *algo =
diff --git a/object-file.h b/object-file.h
index f44758c4f8..53c0e6df09 100644
--- a/object-file.h
+++ b/object-file.h
@@ -92,7 +92,7 @@ int format_object_header(char *str, size_t size, enum object_type type,
  * what we expected, but it might also indicate another error.
  */
 int check_object_signature(struct repository *r, const struct object_id *oid,
-			   void *map, unsigned long size,
+			   void *map, size_t size,
 			   enum object_type type);
 
 /**
-- 
2.53.0

[PATCH 2/4] fsck: use size_t for the size of objects to check

[Elia Pinto]

fsck functions such as fsck_object() and fsck_buffer() take the object
size as an "unsigned long", while some callers pass a size_t. On
platforms where "long" is 32 bits, this truncates the size for objects
of 4GiB or more.

When checking large objects on those platforms, fsck inspects only a
prefix of the buffer.

Use size_t for the size parameter across the fsck functions and internal
helpers, including the loop index in verify_headers().

Some callers and pack verification paths still pass "unsigned long" to
fsck_object(), and fsck_tree() passes the size to tree-walk routines;
those will be updated separately.

Triggering this requires processing 4GiB or more of input on a platform
where "long" is 32 bits, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 fsck.c | 18 +++++++++---------
 fsck.h |  6 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fsck.c b/fsck.c
index 94c8651c7d..7b0e9a5892 100644
--- a/fsck.c
+++ b/fsck.c
@@ -614,7 +614,7 @@ static int verify_ordered(unsigned mode1, const char *name1,
 }
 
 static int fsck_tree(const struct object_id *tree_oid,
-		     const char *buffer, unsigned long size,
+		     const char *buffer, size_t size,
 		     struct fsck_options *options)
 {
 	int retval = 0;
@@ -826,12 +826,12 @@ static int fsck_tree(const struct object_id *tree_oid,
  *     case hit the newline we found here as the end-of-header. This makes it
  *     OK for them to use helpers like parse_oid_hex(), or even skip_prefix().
  */
-static int verify_headers(const void *data, unsigned long size,
+static int verify_headers(const void *data, size_t size,
 			  const struct object_id *oid, enum object_type type,
 			  struct fsck_options *options)
 {
 	const char *buffer = (const char *)data;
-	unsigned long i;
+	size_t i;
 
 	for (i = 0; i < size; i++) {
 		switch (buffer[i]) {
@@ -948,7 +948,7 @@ static int fsck_ident(const char **ident, const char *ident_end,
 }
 
 static int fsck_commit(const struct object_id *oid,
-		       const char *buffer, unsigned long size,
+		       const char *buffer, size_t size,
 		       struct fsck_options *options)
 {
 	struct object_id tree_oid, parent_oid;
@@ -1010,7 +1010,7 @@ static int fsck_commit(const struct object_id *oid,
 }
 
 static int fsck_tag(const struct object_id *oid, const char *buffer,
-		    unsigned long size, struct fsck_options *options)
+		    size_t size, struct fsck_options *options)
 {
 	struct object_id tagged_oid;
 	int tagged_type;
@@ -1019,7 +1019,7 @@ static int fsck_tag(const struct object_id *oid, const char *buffer,
 }
 
 int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
-			unsigned long size, struct fsck_options *options,
+			size_t size, struct fsck_options *options,
 			struct object_id *tagged_oid,
 			int *tagged_type)
 {
@@ -1182,7 +1182,7 @@ static int fsck_gitmodules_fn(const char *var, const char *value,
 }
 
 static int fsck_blob(const struct object_id *oid, const char *buf,
-		     unsigned long size, struct fsck_options *options)
+		     size_t size, struct fsck_options *options)
 {
 	int ret = 0;
 
@@ -1251,7 +1251,7 @@ static int fsck_blob(const struct object_id *oid, const char *buf,
 	return ret;
 }
 
-int fsck_object(struct object *obj, void *data, unsigned long size,
+int fsck_object(struct object *obj, void *data, size_t size,
 	struct fsck_options *options)
 {
 	if (!obj)
@@ -1261,7 +1261,7 @@ int fsck_object(struct object *obj, void *data, unsigned long size,
 }
 
 int fsck_buffer(const struct object_id *oid, enum object_type type,
-		const void *data, unsigned long size,
+		const void *data, size_t size,
 		struct fsck_options *options)
 {
 	if (type == OBJ_BLOB)
diff --git a/fsck.h b/fsck.h
index e77935c8a9..2a075a5572 100644
--- a/fsck.h
+++ b/fsck.h
@@ -196,7 +196,7 @@ int fsck_walk(struct object *obj, void *data, struct fsck_options *options);
  * Blob objects my pass a NULL data pointer, which indicates they are too large
  * to fit in memory. All other types must pass a real buffer.
  */
-int fsck_object(struct object *obj, void *data, unsigned long size,
+int fsck_object(struct object *obj, void *data, size_t size,
 	struct fsck_options *options);
 
 /*
@@ -204,7 +204,7 @@ int fsck_object(struct object *obj, void *data, unsigned long size,
  * struct.
  */
 int fsck_buffer(const struct object_id *oid, enum object_type,
-		const void *data, unsigned long size,
+		const void *data, size_t size,
 		struct fsck_options *options);
 
 /*
@@ -212,7 +212,7 @@ int fsck_buffer(const struct object_id *oid, enum object_type,
  * exposed fsck_object() internals for git-mktag(1).
  */
 int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
-			unsigned long size, struct fsck_options *options,
+			size_t size, struct fsck_options *options,
 			struct object_id *tagged_oid,
 			int *tag_type);
 
-- 
2.53.0

[PATCH 3/4] object, commit, tag: use size_t when parsing object buffers

[Elia Pinto]

Functions like parse_object_buffer(), parse_commit_buffer(),
parse_tag_buffer(), and decode_tree_entry_raw() take the buffer size as
an "unsigned long", while callers pass a size_t. On platforms where
"long" is 32 bits, this truncates the size for objects of 4GiB or more.

When parsing large objects on those platforms, only a prefix of the
buffer is parsed. For example, a signature placed at the end of a large
tag can be missed.

Use size_t for the size parameter across these parsing functions. For
set_commit_buffer(), the commit cache slab still stores "unsigned long"
sizes due to accessors like repo_get_commit_buffer() handing out the
size through an "unsigned long *" output parameter. Therefore,
set_commit_buffer() narrows the size with cast_size_t_to_ulong(), dying
on objects of 4GiB or more on 32-bit long platforms instead of caching a
truncated size, mirroring the behavior already implemented in
repo_get_commit_buffer().

parse_tree_buffer() is changed separately.

Triggering this requires processing 4GiB or more of input on a platform
where "long" is 32 bits, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 commit.c              | 8 ++++----
 commit.h              | 6 +++---
 object-file-convert.c | 2 +-
 object.c              | 2 +-
 object.h              | 2 +-
 tag.c                 | 2 +-
 tag.h                 | 2 +-
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/commit.c b/commit.c
index ad26f0b40a..a94acf4e6f 100644
--- a/commit.c
+++ b/commit.c
@@ -366,12 +366,12 @@ void free_commit_buffer_slab(struct buffer_slab *bs)
 	free(bs);
 }
 
-void set_commit_buffer(struct repository *r, struct commit *commit, void *buffer, unsigned long size)
+void set_commit_buffer(struct repository *r, struct commit *commit, void *buffer, size_t size)
 {
 	struct commit_buffer *v = buffer_slab_at(
 		r->parsed_objects->buffer_slab, commit);
 	v->buffer = buffer;
-	v->size = size;
+	v->size = cast_size_t_to_ulong(size);
 }
 
 const void *get_cached_commit_buffer(struct repository *r, const struct commit *commit, unsigned long *sizep)
@@ -513,7 +513,7 @@ const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)
 	return ret;
 }
 
-int parse_commit_buffer(struct repository *r, struct commit *item, const void *buffer, unsigned long size, int check_graph)
+int parse_commit_buffer(struct repository *r, struct commit *item, const void *buffer, size_t size, int check_graph)
 {
 	const char *tail = buffer;
 	const char *bufptr = buffer;
@@ -1195,7 +1195,7 @@ int parse_signed_commit(const struct commit *commit,
 }
 
 int parse_buffer_signed_by_header(const char *buffer,
-				  unsigned long size,
+				  size_t size,
 				  struct strbuf *payload,
 				  struct strbuf *signature,
 				  const struct git_hash_algo *algop)
diff --git a/commit.h b/commit.h
index 1061ed791b..6a6ff25e12 100644
--- a/commit.h
+++ b/commit.h
@@ -92,7 +92,7 @@ struct commit *lookup_commit_reference_by_name_gently(const char *name,
  */
 struct commit *lookup_commit_or_die(const struct object_id *oid, const char *ref_name);
 
-int parse_commit_buffer(struct repository *r, struct commit *item, const void *buffer, unsigned long size, int check_graph);
+int parse_commit_buffer(struct repository *r, struct commit *item, const void *buffer, size_t size, int check_graph);
 int repo_parse_commit_internal(struct repository *r, struct commit *item,
 			       int quiet_on_missing, int use_commit_graph);
 int repo_parse_commit_gently(struct repository *r,
@@ -131,7 +131,7 @@ void free_commit_buffer_slab(struct buffer_slab *bs);
  * Associate an object buffer with the commit. The ownership of the
  * memory is handed over to the commit, and must be free()-able.
  */
-void set_commit_buffer(struct repository *r, struct commit *, void *buffer, unsigned long size);
+void set_commit_buffer(struct repository *r, struct commit *, void *buffer, size_t size);
 
 /*
  * Get any cached object buffer associated with the commit. Returns NULL
@@ -381,7 +381,7 @@ int run_commit_hook(int editor_is_used, const char *index_file,
 
 /* Parse the signature out of a header. */
 int parse_buffer_signed_by_header(const char *buffer,
-				  unsigned long size,
+				  size_t size,
 				  struct strbuf *payload,
 				  struct strbuf *signature,
 				  const struct git_hash_algo *algop);
diff --git a/object-file-convert.c b/object-file-convert.c
index 63ee18630b..ead88c1021 100644
--- a/object-file-convert.c
+++ b/object-file-convert.c
@@ -51,7 +51,7 @@ int repo_oid_to_algop(struct repository *repo, const struct object_id *srcoid,
 
 static int decode_tree_entry_raw(struct object_id *oid, const char **path,
 				 size_t *len, const struct git_hash_algo *algo,
-				 const char *buf, unsigned long size)
+				 const char *buf, size_t size)
 {
 	uint16_t mode;
 	const unsigned hashsz = algo->rawsz;
diff --git a/object.c b/object.c
index 97f7fc0e87..541ac90dc6 100644
--- a/object.c
+++ b/object.c
@@ -258,7 +258,7 @@ enum peel_status peel_object(struct repository *r,
 	return peel_object_ext(r, name, oid, flags, &dummy);
 }
 
-struct object *parse_object_buffer(struct repository *r, const struct object_id *oid, enum object_type type, unsigned long size, void *buffer, int *eaten_p)
+struct object *parse_object_buffer(struct repository *r, const struct object_id *oid, enum object_type type, size_t size, void *buffer, int *eaten_p)
 {
 	struct object *obj;
 	*eaten_p = 0;
diff --git a/object.h b/object.h
index 46cade33fb..9bf23321a7 100644
--- a/object.h
+++ b/object.h
@@ -241,7 +241,7 @@ struct object *parse_object_or_die(struct repository *repo, const struct object_
  * parsing it.  eaten_p indicates if the object has a borrowed copy
  * of buffer and the caller should not free() it.
  */
-struct object *parse_object_buffer(struct repository *r, const struct object_id *oid, enum object_type type, unsigned long size, void *buffer, int *eaten_p);
+struct object *parse_object_buffer(struct repository *r, const struct object_id *oid, enum object_type type, size_t size, void *buffer, int *eaten_p);
 
 /*
  * Allocate and return an object struct, even if you do not know the type of
diff --git a/tag.c b/tag.c
index 1a00ded6eb..414681c64f 100644
--- a/tag.c
+++ b/tag.c
@@ -127,7 +127,7 @@ void release_tag_memory(struct tag *t)
 	t->date = 0;
 }
 
-int parse_tag_buffer(struct repository *r, struct tag *item, const void *data, unsigned long size)
+int parse_tag_buffer(struct repository *r, struct tag *item, const void *data, size_t size)
 {
 	struct object_id oid;
 	char type[20];
diff --git a/tag.h b/tag.h
index 534687c4ca..a5f0f06c82 100644
--- a/tag.h
+++ b/tag.h
@@ -12,7 +12,7 @@ struct tag {
 	timestamp_t date;
 };
 struct tag *lookup_tag(struct repository *r, const struct object_id *oid);
-int parse_tag_buffer(struct repository *r, struct tag *item, const void *data, unsigned long size);
+int parse_tag_buffer(struct repository *r, struct tag *item, const void *data, size_t size);
 int parse_tag(struct repository *r, struct tag *item);
 void release_tag_memory(struct tag *t);
 struct object *deref_tag(struct repository *r, struct object *, const char *, int);
-- 
2.53.0

[PATCH 4/4] tag, fmt-merge-msg, ref-filter: use size_t for message buffers

[Elia Pinto]

Functions like run_gpg_verify(), fmt_tag_signature(), copy_subject(),
and append_lines() take the length of a tag or message as an "unsigned
long", while callers pass a size_t. On platforms where "long" is 32
bits, this truncates the length for objects of 4GiB or more.

When handling large tags or messages on those platforms, "git tag -v"
inspects only a prefix and fails to find the signature placed at the end
of the tag. Additionally, in copy_subject(), the loop index is an "int",
causing an overflow on subjects of 2GiB or more across all platforms.

Use size_t for the length parameters and the loop index.

Triggering this requires processing 2GiB or 4GiB or more of input, so no
test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 fmt-merge-msg.c | 2 +-
 ref-filter.c    | 6 +++---
 tag.c           | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 14441f23ae..5a96c50383 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -505,7 +505,7 @@ static void fmt_merge_msg_title(struct strbuf *out,
 static void fmt_tag_signature(struct strbuf *tagbuf,
 			      struct strbuf *sig,
 			      const char *buf,
-			      unsigned long len)
+			      size_t len)
 {
 	const char *tag_body = strstr(buf, "\n\n");
 	if (tag_body) {
diff --git a/ref-filter.c b/ref-filter.c
index bdf54f6f59..8e2dcb74b0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1657,10 +1657,10 @@ static const char *copy_email(const char *buf, struct used_atom *atom)
 	return xmemdupz(email, eoemail - email);
 }
 
-static char *copy_subject(const char *buf, unsigned long len)
+static char *copy_subject(const char *buf, size_t len)
 {
 	struct strbuf sb = STRBUF_INIT;
-	int i;
+	size_t i;
 
 	for (i = 0; i < len; i++) {
 		if (buf[i] == '\r' && i + 1 < len && buf[i + 1] == '\n')
@@ -1952,7 +1952,7 @@ static void find_subpos(const char *buf,
  * If 'lines' is greater than 0, append that many lines from the given
  * 'buf' of length 'size' to the given strbuf.
  */
-static void append_lines(struct strbuf *out, const char *buf, unsigned long size, int lines)
+static void append_lines(struct strbuf *out, const char *buf, size_t size, int lines)
 {
 	int i;
 	const char *sp, *eol;
diff --git a/tag.c b/tag.c
index 414681c64f..8709e8c46d 100644
--- a/tag.c
+++ b/tag.c
@@ -16,7 +16,7 @@
 
 const char *tag_type = "tag";
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+static int run_gpg_verify(const char *buf, size_t size, unsigned flags)
 {
 	struct signature_check sigc;
 	struct strbuf payload = STRBUF_INIT;
-- 
2.53.0

Re: [PATCH 0/4] use size_t when parsing and checking objects

[Simon Richter]

[-- Attachment #1.1: Type: text/plain, Size: 1079 bytes --]

Hi,

On 10/31/26 20:09, Elia Pinto wrote:

> This series continues the search for truncated sizes on platforms where
> "long" is 32 bits (Windows). It covers the functions that parse, verify,
> and check whole objects in memory: they take the size as an "unsigned
> long", while their callers have a size_t, so above 4GiB the size gets
> truncated.
Again, reminder that size_t is also 32 bit on 32-bit architectures, 
because size_t is used for the size of in-memory objects.

Anything that is on disk can have a size that only fits into an off_t, 
which is 64 bit if large file support is enabled, and 32 bit if it is 
not. If large file support is disabled, opening a file larger than 2 GB 
is supposed to fail (because you need to pass O_LARGEFILE, which is 
defined only if large file support is enabled), but there is a chance 
that an on-disk object got truncated, so that is not guaranteed.

So, use size_t only for in-memory data, but if you are reading data from 
disk piecewise, the total count of data processed needs to be an off_t.

    Simon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]