[PATCH 0/5] Address overflows pointed out by Coverity

[PATCH 0/5] Address overflows pointed out by Coverity

[Johannes Schindelin via GitGitGadget]

This patch series is part of my effort to bring Git for Windows' Coverity
reports down to zero; Previous patch series have been merged in 347fe8a91b5
(Merge branch 'js/coverity-fixes', 2026-07-16), 0b75ef5168c (Merge branch
'js/coverity-fixes-null-safety', 2026-07-19), fcc8c931b53 (Merge branch
'js/coverity-unchecked-returns-fix', 2026-08-24) and 8bc9c7fcfd8 (Merge
branch 'js/coverity-fixes', 2026-10-03).

Johannes Schindelin (5):
  archive-tar: guard get_path_prefix against zero-length input
  http: use `size_t` for byte position in fwrite_sha1_file
  wrapper: reject insanely large byte counts when reading/writing
  git-compat-util: add `signed_mult_overflows` and `mult_overflows`
    macros
  date: avoid overflow in approxidate relative-time multiplication

 archive-tar.c     |  2 ++
 date.c            | 26 +++++++++++++++++++++--
 git-compat-util.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++
 http.c            |  2 +-
 wrapper.c         | 10 +++++++++
 5 files changed, 91 insertions(+), 3 deletions(-)


base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2253%2Fdscho%2Fcoverity-fixes-overflow-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2253/dscho/coverity-fixes-overflow-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2253
-- 
gitgitgadget

[PATCH 1/5] archive-tar: guard get_path_prefix against zero-length input

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The `get_path_prefix()` function uses a `do ... while()` loop that
unconditionally decrements the `size_t` index variable before checking
the loop condition. When that variable is 0 on entry, the decrement
wraps to `SIZE_MAX`, and `path[SIZE_MAX]` is an out-of-bounds read.

The current caller (`write_tar_entry()`) only invokes this function when
`pathlen > sizeof(header.name)` i.e. when `pathlen` is at least 100.
Therefore, the bug is unreachable in practice.

However, the function's interface accepts arbitrary pathlen and maxlen
values, and a future caller or a maxlen of 0 could trigger the wrap, so
let's add a check for that.

Pointed out by Coverity.

Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 archive-tar.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/archive-tar.c b/archive-tar.c
index a1c66024d4..2da6e55484 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -210,6 +210,8 @@ static size_t get_path_prefix(const char *path, size_t pathlen, size_t maxlen)
 		i--;
 	if (i > maxlen)
 		i = maxlen;
+	if (!i)
+		return 0;
 	do {
 		i--;
 	} while (i > 0 && path[i] != '/');
-- 
gitgitgadget

[PATCH 2/5] http: use `size_t` for byte position in fwrite_sha1_file

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The `fwrite_sha1_file()` function tracks the number of bytes written via
an `int` variable, but the total size to write is of type `size_t`. When
the HTTP response body exceeds INT_MAX (~2GB), this causes an overflow
and a heap under-read, then writes that data to the local object file.

The hash check afterward would reject the corrupt object (so no bad data
enters the object store), but the under-read itself could leak adjacent
heap contents to the temporary file. In practice this is unlikely
because curl delivers data in small chunks (typically 16KB per callback
invocation), so in practice, the total size is far below INT_MAX in a
single call. However, the function interface accepts arbitrary sizes,
and the accumulation across the loop means that the `int` variable grows
across calls within a single invocation, not across callbacks.

Change the type from `int` to `size_t` to match the type of the total
size. This is a hardening fix: the attack surface (a malicious HTTP
server triggering a >2GB single-callback delivery) is theoretical under
normal curl configurations, but the type mismatch is a genuine defect.

Pointed out by Coverity.

Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 http.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/http.c b/http.c
index 4af3c29c76..809471f436 100644
--- a/http.c
+++ b/http.c
@@ -2848,7 +2848,7 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb,
 {
 	unsigned char expn[4096];
 	size_t size = eltsize * nmemb;
-	int posn = 0;
+	size_t posn = 0;
 	struct http_object_request *freq = data;
 	struct active_request_slot *slot = freq->slot;
 
-- 
gitgitgadget

[PATCH 3/5] wrapper: reject insanely large byte counts when reading/writing

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Both the `read_in_full()` as well as the `write_in_full()` function
accumulate the bytes transferred count in a variable of type `ssize_t`,
but accept a `size_t` count parameter. When count exceeds `SSIZE_MAX`,
that total count overflows after processing `SSIZE_MAX` bytes, producing
a negative return value that callers interpret as an error (or worse, a
small positive value after further wrapping).

In practice, git never passes such large counts because file sizes and
buffer allocations are bounded well below SSIZE_MAX on all target
platforms. The POSIX specification for `read()` and `write()` also
limits single operations to `SSIZE_MAX` bytes, and git's
`xread()`/`xwrite()` wrappers further clamp to `MAX_IO_SIZE`. But the
accumulation in the `*_in_full()` wrappers means that even with per-call
clamping, total can exceed `SSIZE_MAX` when count does.

Reject a total count that exceeds the range of `ssize_t` upfront with
`EINVAL` rather than silently producing wrong return values, matching
the POSIX convention for invalid arguments.

Pointed out by Coverity.

Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-compat-util.h |  4 ++++
 wrapper.c         | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index a0f901ce79..afce8b85b8 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -740,6 +740,10 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
  */
 #define DEFAULT_IO_BUFFER_SIZE (128 * 1024)
 
+#ifndef SSIZE_MAX
+# define SSIZE_MAX ((ssize_t)(((size_t)-1) >> 1))
+#endif
+
 #ifdef HAVE_ALLOCA_H
 # include <alloca.h>
 # define xalloca(size)      (alloca(size))
diff --git a/wrapper.c b/wrapper.c
index 05a9cd369c..ea9c14f834 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -288,6 +288,11 @@ ssize_t read_in_full(int fd, void *buf, size_t count)
 	char *p = buf;
 	ssize_t total = 0;
 
+	if (count > SSIZE_MAX) {
+		errno = EINVAL;
+		return -1;
+	}
+
 	while (count > 0) {
 		ssize_t loaded = xread(fd, p, count);
 		if (loaded < 0)
@@ -307,6 +312,11 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
 	const char *p = buf;
 	ssize_t total = 0;
 
+	if (count > SSIZE_MAX) {
+		errno = EINVAL;
+		return -1;
+	}
+
 	while (count > 0) {
 		ssize_t written = xwrite(fd, p, count);
 		if (written < 0)
-- 
gitgitgadget

[PATCH 4/5] git-compat-util: add `signed_mult_overflows` and `mult_overflows` macros

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

`unsigned_mult_overflows()` does not cover signed arithmetic, but types
such as `time_t` can be signed or unsigned. Add signed and type-generic
guards so callers can check multiplication overflow in either case.

Overflow depends on the product type selected by C's usual arithmetic
conversions. Products equal to the signed minimum are representable and
must not be rejected.

Assisted-by: Claude Opus 4.6, GPT-6 and GPT-6.1
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-compat-util.h | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index afce8b85b8..077fa94873 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,9 +85,27 @@ struct strbuf;
 
 #define bitsizeof(x)  (CHAR_BIT * sizeof(x))
 
+#if GIT_GNUC_PREREQ(3, 1)
+#define is_unsigned_type(a) ((__typeof__(a))-1 >= 0)
+#elif __STDC_VERSION__ >= 201112L
+#define is_unsigned_type(a) _Generic((a), \
+	unsigned char: 1, \
+	unsigned short: 1, \
+	unsigned int: 1, \
+	unsigned long: 1, \
+	unsigned long long: 1, \
+	_Bool: 1, \
+	char: (CHAR_MIN == 0), \
+	default: 0)
+#endif
+
 #define maximum_signed_value_of_type(a) \
     (INTMAX_MAX >> (bitsizeof(intmax_t) - bitsizeof(a)))
 
+/* Account for the extra negative value on two's-complement targets. */
+#define minimum_signed_value_of_type(a) \
+    (-maximum_signed_value_of_type(a) - (INTMAX_MIN < -INTMAX_MAX))
+
 #define maximum_unsigned_value_of_type(a) \
     (UINTMAX_MAX >> (bitsizeof(uintmax_t) - bitsizeof(a)))
 
@@ -111,6 +129,38 @@ struct strbuf;
 #define unsigned_mult_overflows(a, b) \
     ((a) && (b) > maximum_unsigned_value_of_type(a) / (a))
 
+/*
+ * Returns true if the multiplication of "a" and "b" will
+ * overflow or underflow a signed type. The types of "a" and "b"
+ * must match and must be signed. Note that this macro evaluates
+ * both "a" and "b" twice!
+ */
+#define signed_mult_overflows(a, b) \
+    (((a) > 0 && (b) > 0 && (a) > maximum_signed_value_of_type(a) / (b)) || \
+     ((a) < 0 && (b) < 0 && (a) < maximum_signed_value_of_type(a) / (b)) || \
+     ((a) > 0 && (b) < 0 && (b) < minimum_signed_value_of_type(a) / (a)) || \
+     ((a) < 0 && (b) > 0 && (a) < minimum_signed_value_of_type(b) / (b)))
+
+/*
+ * Returns true if the product of "a" and "b", after the usual arithmetic
+ * conversions, cannot be represented in the multiplication expression's
+ * type. The operands must be integers and may be evaluated more than once.
+ */
+#if GIT_GNUC_PREREQ(3, 1)
+#define mult_overflows(a, b) \
+    (is_unsigned_type((a) * (b)) \
+     ? unsigned_mult_overflows((__typeof__((a) * (b)))(a), \
+			       (__typeof__((a) * (b)))(b)) \
+     : signed_mult_overflows((__typeof__((a) * (b)))(a), \
+			     (__typeof__((a) * (b)))(b)))
+#else
+/*
+ * This implementation needs GNU `typeof` to convert both operands to the
+ * multiplication's type. Skip the check without that extension.
+ */
+#define mult_overflows(a, b) 0
+#endif
+
 /*
  * Returns true if the left shift of "a" by "shift" bits will
  * overflow. The type of "a" must be unsigned.
-- 
gitgitgadget

[PATCH 5/5] date: avoid overflow in approxidate relative-time multiplication

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The `approxidate_alpha()` function multiplies a time unit length (e.g.,
604800 for weeks) by a user-supplied number to compute a time offset in
seconds. Both operands are `int`, and the product is passed to
`update_tm()` as `time_t`. For inputs like "99999 weeks ago", the int
multiplication overflows (604800 * 99999 is roughly 60 billion, far
beyond `INT_MAX`), producing a wrapped value that results in a wrong
date.

Use the checked `time_t` multiplication so oversized products do not
overflow or wrap around.

On platforms where `time_t` is unsigned (and where the pre-processor
allows us to determine this fact), reject counts that are too large and
report a parse error. Narrowing them first could otherwise produce
negative or truncated values that become bogus offsets. Leave signed
`time_t` count parsing unchanged.

Pointed out by Coverity.

Assisted-by: Claude Opus 4.6 and GPT-6.1 Sol
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 date.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/date.c b/date.c
index 014065b419..0d65804d2f 100644
--- a/date.c
+++ b/date.c
@@ -1298,7 +1298,18 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	while (tl->type) {
 		size_t len = strlen(tl->type);
 		if (match_string(date, tl->type) >= len-1) {
-			update_tm(tm, now, tl->length * *num);
+			time_t length = tl->length, num_t = *num;
+			time_t seconds;
+#ifndef is_unsigned_type
+			seconds = length * num_t;
+#else
+			time_t max_val = is_unsigned_type(length)
+				? maximum_unsigned_value_of_type(length)
+				: maximum_signed_value_of_type(length);
+			seconds = mult_overflows(length, num_t)
+				? max_val : length * num_t;
+#endif
+			update_tm(tm, now, seconds);
 			*num = 0;
 			*touched = 1;
 			return end;
@@ -1368,8 +1379,15 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num,
 	}
 
 	/* Accept zero-padding only for small numbers ("Dec 02", never "Dec 0002") */
-	if (date[0] != '0' || end - date <= 2)
+	if (date[0] != '0' || end - date <= 2) {
+#ifdef is_unsigned_type
+		if (is_unsigned_type((time_t)0) &&
+		    (number > INT_MAX ||
+		     number > maximum_unsigned_value_of_type((time_t)0)))
+			return NULL;
+#endif
 		*num = number;
+	}
 	return end;
 }
 
@@ -1398,6 +1416,10 @@ static timestamp_t approxidate_str(const char *date,
 		if (isdigit(c)) {
 			pending_number(&tm, &number);
 			date = approxidate_digit(date-1, &tm, &number, time_sec);
+			if (!date) {
+				*error_ret = 1;
+				return 0;
+			}
 			touched = 1;
 			continue;
 		}
-- 
gitgitgadget

Re: [PATCH 3/5] wrapper: reject insanely large byte counts when reading/writing

[Phillip Wood]

Hi Johannes

On 14/10/2026 07:09, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> Both the `read_in_full()` as well as the `write_in_full()` function
> accumulate the bytes transferred count in a variable of type `ssize_t`,
> but accept a `size_t` count parameter. When count exceeds `SSIZE_MAX`,
> that total count overflows after processing `SSIZE_MAX` bytes, producing
> a negative return value that callers interpret as an error (or worse, a
> small positive value after further wrapping).
> 
> In practice, git never passes such large counts because file sizes and
> buffer allocations are bounded well below SSIZE_MAX on all target
> platforms. 

Are we sure about that? In the past we've had bug fixes such as 
c14e5a1a50 (transport-helper: use xread instead of read, 2019-01-03)
on platforms where ssize_t is narrower that size_t (POSIX does not 
require them to be the same width). I've cc'd Randall to see if ssize_t 
is still narrower than size_t on the NonStop platforms git supports.

Thanks

Phillip

> The POSIX specification for `read()` and `write()` also
> limits single operations to `SSIZE_MAX` bytes, and git's
> `xread()`/`xwrite()` wrappers further clamp to `MAX_IO_SIZE`. But the
> accumulation in the `*_in_full()` wrappers means that even with per-call
> clamping, total can exceed `SSIZE_MAX` when count does.
> 
> Reject a total count that exceeds the range of `ssize_t` upfront with
> `EINVAL` rather than silently producing wrong return values, matching
> the POSIX convention for invalid arguments.
> 
> Pointed out by Coverity.
> 
> Assisted-by: Claude Opus 4.6
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>   git-compat-util.h |  4 ++++
>   wrapper.c         | 10 ++++++++++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/git-compat-util.h b/git-compat-util.h
> index a0f901ce79..afce8b85b8 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -740,6 +740,10 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
>    */
>   #define DEFAULT_IO_BUFFER_SIZE (128 * 1024)
>   
> +#ifndef SSIZE_MAX
> +# define SSIZE_MAX ((ssize_t)(((size_t)-1) >> 1))
> +#endif
> +
>   #ifdef HAVE_ALLOCA_H
>   # include <alloca.h>
>   # define xalloca(size)      (alloca(size))
> diff --git a/wrapper.c b/wrapper.c
> index 05a9cd369c..ea9c14f834 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -288,6 +288,11 @@ ssize_t read_in_full(int fd, void *buf, size_t count)
>   	char *p = buf;
>   	ssize_t total = 0;
>   
> +	if (count > SSIZE_MAX) {
> +		errno = EINVAL;
> +		return -1;
> +	}
> +
>   	while (count > 0) {
>   		ssize_t loaded = xread(fd, p, count);
>   		if (loaded < 0)
> @@ -307,6 +312,11 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
>   	const char *p = buf;
>   	ssize_t total = 0;
>   
> +	if (count > SSIZE_MAX) {
> +		errno = EINVAL;
> +		return -1;
> +	}
> +
>   	while (count > 0) {
>   		ssize_t written = xwrite(fd, p, count);
>   		if (written < 0)