[PATCH 0/7] Fix issues pointed out in Git for Windows by Coverity after merging v2.56.0-rc0

[PATCH 0/7] Fix issues pointed out in Git for Windows by Coverity after merging v2.56.0-rc0

[Johannes Schindelin via GitGitGadget]

These Coverity reports are new as of this -rc cycle; Apart from the writev
one, I don't think any of these are pressing, in most cases I am still
puzzled why they were reported only now.

Johannes Schindelin (7):
  wrapper: guard writev_in_full() against signed overflow
  gpg-interface: make signature-prefix matching length-aware
  midx: validate incremental MIDX pack IDs
  rerere: do not record failed conflict resolution data
  t/unit-tests: check reftable iterator initialization
  oss-fuzz: handle reftable iterator initialization failures
  test-read-midx: check midx_fill_entry() result

 gpg-interface.c                 | 10 +++++-----
 midx.c                          | 12 +++++++++---
 oss-fuzz/fuzz-reftable.c        | 18 ++++++++++--------
 rerere.c                        | 30 ++++++++++++++++++++++++++----
 t/helper/test-read-midx.c       |  6 +++++-
 t/unit-tests/u-reftable-table.c |  2 +-
 wrapper.c                       |  4 ++++
 7 files changed, 60 insertions(+), 22 deletions(-)


base-commit: 12cb6293d6288865c1a133cf22accbaf99d13eb6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2231%2Fdscho%2Ffix-coverity-high-severity-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2231/dscho/fix-coverity-high-severity-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2231
-- 
gitgitgadget

[PATCH 1/7] wrapper: guard writev_in_full() against signed overflow

[Johannes Schindelin via GitGitGadget]

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

As Git for Windows' Coverity run after merging v2.56.0-rc0 reported,
`writev_in_full()` keeps its cumulative successful output in an
`ssize_t`. Although `xwritev()` limits each individual write to a
syscall-sized amount, repeated successful writes can still exceed
`SSIZE_MAX`. The unchecked accumulation was introduced by d70eb7f3600d
(wrapper: introduce writev(3p) wrappers, 2026-08-07).

Treat an aggregate that would overflow the signed total as an I/O
failure.

Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 wrapper.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/wrapper.c b/wrapper.c
index 561f9ee9c9..05a9cd369c 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -376,6 +376,10 @@ ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)
 			return -1;
 		}
 
+		if (signed_add_overflows(total_written, bytes_written)) {
+			errno = EOVERFLOW;
+			return -1;
+		}
 		total_written += bytes_written;
 
 		/*
-- 
gitgitgadget

[PATCH 2/7] gpg-interface: make signature-prefix matching length-aware

[Johannes Schindelin via GitGitGadget]

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

After merging v2.56.0-rc0 into Git for Windows, its Coverity run
reported the following issue: The `parse_signed_buffer()` function
accepts object buffers with an explicit size, while
`get_format_by_sig()` uses `starts_with()`, i.e. it expects a
NUL-terminated buffer. A tag object with a non-NUL-terminated payload
ending in a partial signature prefix, such as a final '-' byte, could
therefore cause an invalid read past the object buffer.

The observable consequences are limited to reading past the allocation.
In practice it can crash Git if the read enters an unmapped page. It can
also misplace the payload/signature split, corrupting the compat-hash
object being written.

The older unbounded matcher predates this path, but c8762c30df5b
(object-file-convert: convert tag objects when writing, 2023-10-01)
exposed the defect by passing exact-sized converted tag buffers to
`parse_signed_buffer()`. That commit first shipped in v2.45.0, so the
defect has been latent in every release since.

This pattern was noticed on the mailing list in February 2024. Reviewing
a patch for a very similar issue in commit.c's find_header_mem(), Jeff
King observed in
https://lore.kernel.org/git/20240208214137.GB1090198@coredump.intra.peff.net/:

  But more interestingly: even though we pass a buf/len pair to
  parse_signed_buffer(), it then calls get_format_by_sig() which takes
  only a NUL-terminated string. [...] That raises the question of
  whether parse_signed_buffer() has a similar walk-too-far problem. ;)
  The answer is no, because we feed it from a strbuf. But it's not a
  great pattern overall.

That reasoning surveyed the callers that existed at the time and missed
c8762c30df5b (object-file-convert: convert tag objects when writing,
2023-10-01), which was four months old at that time, and does not feed
from a strbuf; `convert_tag_object()` hands `parse_signed_buffer()` an
exact-sized `xmalloc()` buffer, and the concern flagged and dismissed in
that thread is exactly the defect Coverity now reports.

Jeff went on to add `starts_with_mem()` a month later, in
https://lore.kernel.org/git/20240307092638.GK2080210@coredump.intra.peff.net/,
precisely for "cases where the buffer is not NUL-terminated (and we
instead have an explicit size or end pointer)", so the tool for this fix
has been in the tree since v2.45.0.

Even though the issue had been latent, it most likely surfaced via
Coverity because of 215d305f450f (odb: compute compat object ID in
`odb_write_object_ext()`, 2026-07-17), which moved
`convert_object_file()` out of the `source->write_object` function
pointer into a direct call in `odb_write_object_ext()`.

Preserve the existing NUL-terminated behavior for callers that provide
strings while making signature-prefix matching honor the known buffer
lengths, via the `starts_with_mem()` helper. This keeps reads within the
object data without implying exploitability beyond the observed invalid
read.

Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 gpg-interface.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 95abf1ef4e..60c315fba9 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -133,20 +133,20 @@ static struct gpg_format *get_format_by_name(const char *str)
 	return NULL;
 }
 
-static struct gpg_format *get_format_by_sig(const char *sig)
+static struct gpg_format *get_format_by_sig(const char *sig, size_t len)
 {
 	int j;
 
 	for (size_t i = 0; i < ARRAY_SIZE(gpg_format); i++)
 		for (j = 0; gpg_format[i].sigs[j]; j++)
-			if (starts_with(sig, gpg_format[i].sigs[j]))
+			if (starts_with_mem(sig, len, gpg_format[i].sigs[j]))
 				return gpg_format + i;
 	return NULL;
 }
 
 const char *get_signature_format(const char *buf)
 {
-	struct gpg_format *format = get_format_by_sig(buf);
+	struct gpg_format *format = get_format_by_sig(buf, strlen(buf));
 	return format ? format->name : "unknown";
 }
 
@@ -669,7 +669,7 @@ int check_signature(struct signature_check *sigc,
 	sigc->result = 'N';
 	sigc->trust_level = TRUST_UNDEFINED;
 
-	fmt = get_format_by_sig(signature);
+	fmt = get_format_by_sig(signature, slen);
 	if (!fmt)
 		die(_("bad/incompatible signature '%s'"), signature);
 
@@ -706,7 +706,7 @@ size_t parse_signed_buffer(const char *buf, size_t size)
 	while (len < size) {
 		const char *eol;
 
-		if (get_format_by_sig(buf + len))
+		if (get_format_by_sig(buf + len, size - len))
 			match = len;
 
 		eol = memchr(buf + len, '\n', size - len);
-- 
gitgitgadget

Re: [PATCH 2/7] gpg-interface: make signature-prefix matching length-aware

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> @@ -669,7 +669,7 @@ int check_signature(struct signature_check *sigc,
>  	sigc->result = 'N';
>  	sigc->trust_level = TRUST_UNDEFINED;
>  
> -	fmt = get_format_by_sig(signature);
> +	fmt = get_format_by_sig(signature, slen);
>  	if (!fmt)
>  		die(_("bad/incompatible signature '%s'"), signature);

All the existing callers of check_signature() pass a NUL-terminated
buffer which is <buf, len> pair of a strbuf.  Another approach that
may be simpler is to drop the slen parameter from check_signature().

Re: [PATCH 2/7] gpg-interface: make signature-prefix matching length-aware

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 4411 bytes --]

Hi Junio,

On Thu, 17 Sep 2026, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > @@ -669,7 +669,7 @@ int check_signature(struct signature_check *sigc,
> >  	sigc->result = 'N';
> >  	sigc->trust_level = TRUST_UNDEFINED;
> >  
> > -	fmt = get_format_by_sig(signature);
> > +	fmt = get_format_by_sig(signature, slen);

This hunk is a direct consequence of `get_format_by_sig()` gaining a
length parameter earlier in the same patch: it now has three callers,
`get_signature_format()`, `check_signature()` here, and the loop inside
`parse_signed_buffer()` (the actual site of the bug this series fixes,
Coverity issue with CID 1678690 if you want to double-check).

Once the function takes a (sig, len) pair uniformly, every one of them has
to pass a length, so this call site is not optional scaffolding; it is
what makes all three callers correct by construction instead of leaving
two of them trusting NUL-termination and one bounds-checked.

> >  	if (!fmt)
> >  		die(_("bad/incompatible signature '%s'"), signature);
> 
> All the existing callers of check_signature() pass a NUL-terminated
> buffer which is <buf, len> pair of a strbuf.

That holds for six of the seven call sites: `commit.c`, `tag.c`,
`builtin/fast-import.c`, `fmt-merge-msg.c`, and `log-tree.c` (twice).
`builtin/receive-pack.c` is a minor wrinkle worth flagging: it passes
`push_cert.buf + bogs` ("bogs" = "beginning_of_gpg_sig") and
`push_cert.len - bogs`, an offset sub-buffer of `push_cert`, not that
strbuf's own buf/len pair verbatim. It still ends on `push_cert`'s own
terminating NUL, so the observation holds in spirit, but strictly the
pattern is "ends at some strbuf's own NUL", which is more a matter of
code-review convention across call sites than something
`check_signature()`'s own signature guarantees.

> Another approach that may be simpler is to drop the slen parameter from
> check_signature().

I would rather keep it right where it is, for (at least 😊) two reasons.

First, `slen` isn't new here: `check_signature()` has taken a `(sigc,
signature, slen)` signature since 02769437e142 (ssh signing: use sigc
struct to pass payload, 2021-12-09), three years before this series, so
dropping it now would fold an unrelated API change into a bug fix.

Second, and this is the one that actually worries me: `slen` is used twice
inside `check_signature()`, not once. Besides the `get_format_by_sig()`
call above, the pre-existing `fmt->verify_signed_buffer(sigc, fmt,
signature, slen)` a few lines down depends on it too (there it is named
`signature_size`). Both concrete implementations of that vtable member,
`verify_gpg_signed_buffer()` and `verify_ssh_signed_buffer()`, use
`signature_size` to decide exactly how many bytes to `write_in_full()`
into the temporary file that then gets handed to `gpg`/`ssh-keygen` as the
detached signature to verify. That is the authoritative byte count of the
blob being verified, not a defensive nicety. If we dropped `slen` and let
`check_signature()` fall back on `strlen(signature)`, a signature blob
with an embedded NUL before its logical end would get truncated before it
ever reaches the external verifier: a correctness regression in the actual
cryptographic verification path, not merely in the prefix-matching helper
this series fixes. `check_signature()` has no doc comment promising
`signature` is free of embedded NULs, so dropping `slen` would trade an
explicit length for an implicit assumption.

As the commit message notes, we have been down this road with this exact
function chain before. In February 2024, Peff concluded there was no
walk-too-far problem in `parse_signed_buffer()` "because we feed it from a
strbuf":
https://lore.kernel.org/git/20240208214137.GB1090198@coredump.intra.peff.net/

But that conclusion was already four months stale: c8762c30df5b
(object-file-convert: convert tag objects when writing, 2023-10-01) had
already added `convert_tag_object()` as a caller that does _not_ feed from
a strbuf: the same gap this series closes. Applying the same "audit
today's callers and assume it holds" reasoning to `check_signature()` now
risks reproducing that failure mode a second time.

So I would like to keep `slen` and the `get_format_by_sig(signature,
slen)` call as in the patch.

Ciao,
Johannes

Re: [PATCH 2/7] gpg-interface: make signature-prefix matching length-aware

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> That holds for six of the seven call sites: `commit.c`, `tag.c`,
> `builtin/fast-import.c`, `fmt-merge-msg.c`, and `log-tree.c` (twice).
> `builtin/receive-pack.c` is a minor wrinkle worth flagging: it passes
> `push_cert.buf + bogs` ("bogs" = "beginning_of_gpg_sig") and
> `push_cert.len - bogs`, an offset sub-buffer of `push_cert`, not that
> strbuf's own buf/len pair verbatim. It still ends on `push_cert`'s own
> terminating NUL, so the observation holds in spirit, but strictly the
> pattern is "ends at some strbuf's own NUL", which is more a matter of
> code-review convention across call sites than something
> `check_signature()`'s own signature guarantees.

Yes but the audit was "is slen our callers pass redundant?", and not
"does everybody pass strbuf and we are better off passing a pionter
to a strbuf?".  And the answer to the former question is "yes".

And I do not quite understand or agree with the logic here.

> ... Applying the same "audit
> today's callers and assume it holds" reasoning to `check_signature()` now
> risks reproducing that failure mode a second time.

What I was saying was to force all current *and* *future* callers to
pass NUL-terminated string by removing slen.

Having said all that, I think this falls into "once the code is
written (and more importantly, once it is reviewed, as that is a lot
more costly part of the development process for machine written
code), it is not worth going back and change it, as the difference
is not large enough either way."

[PATCH 3/7] midx: validate incremental MIDX pack IDs

[Johannes Schindelin via GitGitGadget]

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

Incremental MIDX support made object-offset pack IDs local to each layer
and then converted them to chain-global IDs by adding
`num_packs_in_base`. The conversion was introduced by 19419821bac5
(midx: teach `nth_midxed_pack_int_id()` about incremental MIDXs,
2024-08-06). Chain-aware pack preparation followed in 1820bd878c62
(midx: teach `prepare_midx_pack()` about incremental MIDXs, 2024-08-06),
but the final `midx_fill_entry()` lookup remained tied to the original
layer. Only with 8f909ff4e9e8 (packfile: recover when a multi-pack-index
names a removed pack, 2026-08-29) did Coverity point out this issue: a
local ID such as `UINT32_MAX` could wrap when the base-pack count was
added, producing a plausible but incorrect global ID. After
`prepare_midx_pack()` resolved the chain, `midx_fill_entry()` could then
underflow or address the wrong layer while indexing the current layer's
pack array, causing an invalid memory access and crashing Git.

Validate each local pack ID against its layer's pack count before adding
the base count, and obtain the final pack through `nth_midxed_pack()`,
which resolves the correct MIDX layer. This prevents an invalid local ID
from wrapping during conversion and ensures that the lookup uses the
layer identified by the resolved chain-global ID.

Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 midx.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/midx.c b/midx.c
index 6d1c548e3d..6968fc1c00 100644
--- a/midx.c
+++ b/midx.c
@@ -583,10 +583,16 @@ off_t nth_midxed_offset(struct multi_pack_index *m, uint32_t pos)
 
 uint32_t nth_midxed_pack_int_id(struct multi_pack_index *m, uint32_t pos)
 {
+	uint32_t pack_int_id;
+
 	pos = midx_for_object(&m, pos);
+	pack_int_id = get_be32(m->chunk_object_offsets +
+			       (off_t)pos * MIDX_CHUNK_OFFSET_WIDTH);
+	if (pack_int_id >= m->num_packs)
+		die(_("bad pack-int-id: %"PRIu32" (%"PRIu32" total packs)"),
+		    pack_int_id, m->num_packs);
 
-	return m->num_packs_in_base + get_be32(m->chunk_object_offsets +
-					       (off_t)pos * MIDX_CHUNK_OFFSET_WIDTH);
+	return m->num_packs_in_base + pack_int_id;
 }
 
 enum midx_fill_result midx_fill_entry(struct multi_pack_index *m,
@@ -606,7 +612,7 @@ enum midx_fill_result midx_fill_entry(struct multi_pack_index *m,
 
 	if (prepare_midx_pack(m, pack_int_id))
 		return MIDX_FILL_OWNER_UNAVAILABLE;
-	p = m->packs[pack_int_id - m->num_packs_in_base];
+	p = nth_midxed_pack(m, pack_int_id);
 
 	/*
 	* We are about to tell the caller where they can locate the
-- 
gitgitgadget

[PATCH 4/7] rerere: do not record failed conflict resolution data

[Johannes Schindelin via GitGitGadget]

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

`rerere` can mark a conflict variant as resolved even when writing its
preimage or postimage fails. A later invocation may then replay
incomplete data from the cache, turning a local filesystem failure into
an incorrect working-tree change.

629716d256a7 (rerere: do use multiple variants, 2015-07-30) introduced
the code paths without checks for those I/O results. Treat such failures
as failures, report them, and leave the rerere status unchanged unless
the corresponding data was recorded successfully.

The defect has been latent since 2015. Git for Windows' Coverity run
only reported it after merging v2.56.0-rc0, for reasons that could not
be figured out in a reasonable amount of time.

Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 rerere.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/rerere.c b/rerere.c
index 1c3745d9e3..45bbe6ab2c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -476,8 +476,11 @@ static int handle_file(struct index_state *istate,
 			unlink_or_warn(output);
 		return error(_("could not parse conflict hunks in '%s'"), path);
 	}
-	if (io.io.wrerror)
+	if (io.io.wrerror) {
+		if (output)
+			unlink_or_warn(output);
 		return -1;
+	}
 	return has_conflicts;
 }
 
@@ -729,8 +732,25 @@ static void do_rerere_one_path(struct index_state *istate,
 
 	/* Has the user resolved it already? */
 	if (variant >= 0) {
-		if (!handle_file(istate, path, NULL, NULL)) {
-			copy_file(the_repository, rerere_path(&buf, id, "postimage"), path, 0666);
+		int ret = handle_file(istate, path, NULL, NULL);
+
+		if (ret < 0)
+			goto out;
+		if (!ret) {
+			const int had_postimage =
+				id->collection->status[variant] & RR_HAS_POSTIMAGE;
+			const char *postimage =
+				rerere_path(&buf, id, "postimage");
+
+			if (copy_file(the_repository,
+				      postimage,
+				      path, 0666)) {
+				if (!had_postimage)
+					unlink_or_warn(postimage);
+				error_errno(_("could not copy resolution for '%s'"),
+					    path);
+				goto out;
+			}
 			id->collection->status[variant] |= RR_HAS_POSTIMAGE;
 			fprintf_ln(stderr, _("Recorded resolution for '%s'."), path);
 			free_rerere_id(rr_item);
@@ -778,7 +798,9 @@ static void do_rerere_one_path(struct index_state *istate,
 	assign_variant(id);
 
 	variant = id->variant;
-	handle_file(istate, path, NULL, rerere_path(&buf, id, "preimage"));
+	if (handle_file(istate, path, NULL,
+			rerere_path(&buf, id, "preimage")) < 0)
+		goto out;
 	if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
 		const char *path = rerere_path(&buf, id, "postimage");
 		if (unlink(path))
-- 
gitgitgadget

Re: [PATCH 4/7] rerere: do not record failed conflict resolution data

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> @@ -778,7 +798,9 @@ static void do_rerere_one_path(struct index_state *istate,
>  	assign_variant(id);
>  
>  	variant = id->variant;
> -	handle_file(istate, path, NULL, rerere_path(&buf, id, "preimage"));
> +	if (handle_file(istate, path, NULL,
> +			rerere_path(&buf, id, "preimage")) < 0)
> +		goto out;
>  	if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
>  		const char *path = rerere_path(&buf, id, "postimage");
>  		if (unlink(path))

Good to see this one, which is the only unchecked call to the
handle_file() function, checked for an error.  Looking good.

Thanks.

[PATCH 5/7] t/unit-tests: check reftable iterator initialization

[Johannes Schindelin via GitGitGadget]

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

Coverity pointed out that the
`test_reftable_table__seek_invalid_log_offset()` test, which was
introduced by a1c085df8dcb (reftable/table: fix NULL pointer access when
seeking to bogus offsets, 2026-07-03), ignores the result of
`reftable_table_init_log_iterator()` and proceeds to
`reftable_iterator_seek_log()`, although initialization can return
`REFTABLE_OUT_OF_MEMORY_ERROR` without installing an ops table. Under
allocation failure, the test then dereferences a NULL function table.

Assert successful iterator initialization before seeking.

Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/unit-tests/u-reftable-table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/unit-tests/u-reftable-table.c b/t/unit-tests/u-reftable-table.c
index bd04b477a3..1e4378b2eb 100644
--- a/t/unit-tests/u-reftable-table.c
+++ b/t/unit-tests/u-reftable-table.c
@@ -257,7 +257,7 @@ void test_reftable_table__seek_invalid_log_offset(void)
 	 * know that the table is corrupt, so the seek must report a format
 	 * error instead of pretending that the section is empty.
 	 */
-	reftable_table_init_log_iterator(table, &it);
+	cl_assert_equal_i(reftable_table_init_log_iterator(table, &it), 0);
 	cl_assert_equal_i(reftable_iterator_seek_log(&it, ""),
 			  REFTABLE_FORMAT_ERROR);
 
-- 
gitgitgadget

[PATCH 6/7] oss-fuzz: handle reftable iterator initialization failures

[Johannes Schindelin via GitGitGadget]

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

The reftable fuzzer introduced by adf45165e65b (oss-fuzz: add fuzzer for
parsing reftables, 2026-07-03) ignored failures from
`reftable_table_init_ref_iterator()` and
`reftable_table_init_log_iterator()`. Coverity reported that under
allocation failure, either constructor can return
`REFTABLE_OUT_OF_MEMORY_ERROR` without installing an ops table, allowing
a subsequent seek to dereference NULL.

Treat iterator initialization failure as a reason to skip the
corresponding seek and iteration while retaining safe destruction for an
uninitialized iterator.

Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 oss-fuzz/fuzz-reftable.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/oss-fuzz/fuzz-reftable.c b/oss-fuzz/fuzz-reftable.c
index c46eac2c6b..75b8ad0c3d 100644
--- a/oss-fuzz/fuzz-reftable.c
+++ b/oss-fuzz/fuzz-reftable.c
@@ -33,10 +33,11 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
 		struct reftable_ref_record ref = { 0 };
 		struct reftable_iterator it = { 0 };
 
-		reftable_table_init_ref_iterator(table, &it);
-		if (!reftable_iterator_seek_ref(&it, ""))
-			while (!reftable_iterator_next_ref(&it, &ref))
-				;
+		if (!reftable_table_init_ref_iterator(table, &it)) {
+			if (!reftable_iterator_seek_ref(&it, ""))
+				while (!reftable_iterator_next_ref(&it, &ref))
+					;
+		}
 
 		reftable_ref_record_release(&ref);
 		reftable_iterator_destroy(&it);
@@ -46,10 +47,11 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
 		struct reftable_log_record log = { 0 };
 		struct reftable_iterator it = { 0 };
 
-		reftable_table_init_log_iterator(table, &it);
-		if (!reftable_iterator_seek_log(&it, ""))
-			while (!reftable_iterator_next_log(&it, &log))
-				;
+		if (!reftable_table_init_log_iterator(table, &it)) {
+			if (!reftable_iterator_seek_log(&it, ""))
+				while (!reftable_iterator_next_log(&it, &log))
+					;
+		}
 
 		reftable_log_record_release(&log);
 		reftable_iterator_destroy(&it);
-- 
gitgitgadget

[PATCH 7/7] test-read-midx: check midx_fill_entry() result

[Johannes Schindelin via GitGitGadget]

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

The `--show-objects` mode of `read_midx_file()` uses the output of
`midx_fill_entry()` without checking whether the lookup succeeded. A
failed lookup or unavailable pack can leave that output unusable,
allowing malformed or concurrently changed MIDX data to make this test
helper crash instead of reporting a controlled error.

Reject the entry unless `midx_fill_entry()` returns `MIDX_FILL_HIT`. The
unchecked call was introduced by 86d174b7246b
(t/helper/test-read-midx.c: add '--show-objects', 2021-03-30); later
incremental-MIDX changes expanded the possible failure modes, but this
remains a test-helper robustness issue, not a production Git attack
surface or an arbitrary-code-execution vulnerability.

It is unclear why Coverity reports this issue in Git for Windows only
after merging v2.56.0-rc0; The issue was not reported before.

Assisted-by: GPT-5.6 Luna
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/helper/test-read-midx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
index 83b07c6236..412089563f 100644
--- a/t/helper/test-read-midx.c
+++ b/t/helper/test-read-midx.c
@@ -90,7 +90,11 @@ static int read_midx_file(const char *object_dir, const char *checksum,
 		for (i = 0; i < m->num_objects; i++) {
 			nth_midxed_object_oid(&oid, m,
 					      i + m->num_objects_in_base);
-			midx_fill_entry(m, &oid, &e, NULL);
+			if (midx_fill_entry(m, &oid, &e, NULL) !=
+			    MIDX_FILL_HIT) {
+				ret = error(_("failed to load pack entry"));
+				goto out;
+			}
 
 			printf("%s %"PRIu64"\t%s\n",
 			       oid_to_hex(&oid), e.offset, e.p->pack_name);
-- 
gitgitgadget