{"thread":{"id":"66341","subject":"[PATCH 0/7] Fix issues pointed out in Git for Windows by Coverity after merging v2.56.0-rc0","startedAt":"2026-09-17T17:52:39Z","lastAt":"2026-09-18T08:59:14Z","messageCount":12,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"552819","messageId":"pull.2231.git.1789667556.gitgitgadget@gmail.com","threadId":"66341","inReplyTo":null,"subject":"[PATCH 0/7] Fix issues pointed out in Git for Windows by Coverity after merging v2.56.0-rc0","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-17T17:52:29Z","receivedAt":"2026-09-17T17:52:39Z","isPatch":true,"body":"These Coverity reports are new as of this -rc cycle; Apart from the writev\none, I don't think any of these are pressing, in most cases I am still\npuzzled why they were reported only now.\n\nJohannes Schindelin (7):\n wrapper: guard writev_in_full() against signed overflow\n gpg-interface: make signature-prefix matching length-aware\n midx: validate incremental MIDX pack IDs\n rerere: do not record failed conflict resolution data\n t/unit-tests: check reftable iterator initialization\n oss-fuzz: handle reftable iterator initialization failures\n test-read-midx: check midx_fill_entry() result\n\n gpg-interface.c | 10 +++++-----\n midx.c | 12 +++++++++---\n oss-fuzz/fuzz-reftable.c | 18 ++++++++++--------\n rerere.c | 30 ++++++++++++++++++++++++++----\n t/helper/test-read-midx.c | 6 +++++-\n t/unit-tests/u-reftable-table.c | 2 +-\n wrapper.c | 4 ++++\n 7 files changed, 60 insertions(+), 22 deletions(-)\n\n\nbase-commit: 12cb6293d6288865c1a133cf22accbaf99d13eb6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2231%2Fdscho%2Ffix-coverity-high-severity-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2231/dscho/fix-coverity-high-severity-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2231\n-- \ngitgitgadget\n"},{"id":"552820","messageId":"ef08bbae2dec527fe2097c09f9f3187e0f38680d.1789667556.git.gitgitgadget@gmail.com","threadId":"66341","inReplyTo":"pull.2231.git.1789667556.gitgitgadget@gmail.com","subject":"[PATCH 1/7] wrapper: guard writev_in_full() against signed overflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-17T17:52:30Z","receivedAt":"2026-09-17T17:52:42Z","isPatch":true,"body":"From: Johannes Schindelin \n\nAs Git for Windows' Coverity run after merging v2.56.0-rc0 reported,\n`writev_in_full()` keeps its cumulative successful output in an\n`ssize_t`. Although `xwritev()` limits each individual write to a\nsyscall-sized amount, repeated successful writes can still exceed\n`SSIZE_MAX`. The unchecked accumulation was introduced by d70eb7f3600d\n(wrapper: introduce writev(3p) wrappers, 2026-08-07).\n\nTreat an aggregate that would overflow the signed total as an I/O\nfailure.\n\nAssisted-by: GPT-5.6 Luna\nSigned-off-by: Johannes Schindelin \n---\n wrapper.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex 561f9ee9c9..05a9cd369c 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -376,6 +376,10 @@ ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)\n \t\t\treturn -1;\n \t\t}\n \n+\t\tif (signed_add_overflows(total_written, bytes_written)) {\n+\t\t\terrno = EOVERFLOW;\n+\t\t\treturn -1;\n+\t\t}\n \t\ttotal_written += bytes_written;\n \n \t\t/*\n-- \ngitgitgadget\n\n"},{"id":"552821","messageId":"3fc7774ba867a10f35f7a74424baa4527f038232.1789667556.git.gitgitgadget@gmail.com","threadId":"66341","inReplyTo":"pull.2231.git.1789667556.gitgitgadget@gmail.com","subject":"[PATCH 2/7] gpg-interface: make signature-prefix matching length-aware","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-17T17:52:31Z","receivedAt":"2026-09-17T17:52:43Z","isPatch":true,"body":"From: Johannes Schindelin \n\nAfter merging v2.56.0-rc0 into Git for Windows, its Coverity run\nreported the following issue: The `parse_signed_buffer()` function\naccepts object buffers with an explicit size, while\n`get_format_by_sig()` uses `starts_with()`, i.e. it expects a\nNUL-terminated buffer. A tag object with a non-NUL-terminated payload\nending in a partial signature prefix, such as a final '-' byte, could\ntherefore cause an invalid read past the object buffer.\n\nThe observable consequences are limited to reading past the allocation.\nIn practice it can crash Git if the read enters an unmapped page. It can\nalso misplace the payload/signature split, corrupting the compat-hash\nobject being written.\n\nThe older unbounded matcher predates this path, but c8762c30df5b\n(object-file-convert: convert tag objects when writing, 2023-10-01)\nexposed the defect by passing exact-sized converted tag buffers to\n`parse_signed_buffer()`. That commit first shipped in v2.45.0, so the\ndefect has been latent in every release since.\n\nThis pattern was noticed on the mailing list in February 2024. Reviewing\na patch for a very similar issue in commit.c's find_header_mem(), Jeff\nKing observed in\nhttps://lore.kernel.org/git/20240208214137.GB1090198@coredump.intra.peff.net/:\n\n But more interestingly: even though we pass a buf/len pair to\n parse_signed_buffer(), it then calls get_format_by_sig() which takes\n only a NUL-terminated string. [...] That raises the question of\n whether parse_signed_buffer() has a similar walk-too-far problem. ;)\n The answer is no, because we feed it from a strbuf. But it's not a\n great pattern overall.\n\nThat reasoning surveyed the callers that existed at the time and missed\nc8762c30df5b (object-file-convert: convert tag objects when writing,\n2023-10-01), which was four months old at that time, and does not feed\nfrom a strbuf; `convert_tag_object()` hands `parse_signed_buffer()` an\nexact-sized `xmalloc()` buffer, and the concern flagged and dismissed in\nthat thread is exactly the defect Coverity now reports.\n\nJeff went on to add `starts_with_mem()` a month later, in\nhttps://lore.kernel.org/git/20240307092638.GK2080210@coredump.intra.peff.net/,\nprecisely for \"cases where the buffer is not NUL-terminated (and we\ninstead have an explicit size or end pointer)\", so the tool for this fix\nhas been in the tree since v2.45.0.\n\nEven though the issue had been latent, it most likely surfaced via\nCoverity because of 215d305f450f (odb: compute compat object ID in\n`odb_write_object_ext()`, 2026-07-17), which moved\n`convert_object_file()` out of the `source->write_object` function\npointer into a direct call in `odb_write_object_ext()`.\n\nPreserve the existing NUL-terminated behavior for callers that provide\nstrings while making signature-prefix matching honor the known buffer\nlengths, via the `starts_with_mem()` helper. This keeps reads within the\nobject data without implying exploitability beyond the observed invalid\nread.\n\nAssisted-by: GPT-5.6 Luna\nSigned-off-by: Johannes Schindelin \n---\n gpg-interface.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 95abf1ef4e..60c315fba9 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -133,20 +133,20 @@ static struct gpg_format *get_format_by_name(const char *str)\n \treturn NULL;\n }\n \n-static struct gpg_format *get_format_by_sig(const char *sig)\n+static struct gpg_format *get_format_by_sig(const char *sig, size_t len)\n {\n \tint j;\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(gpg_format); i++)\n \t\tfor (j = 0; gpg_format[i].sigs[j]; j++)\n-\t\t\tif (starts_with(sig, gpg_format[i].sigs[j]))\n+\t\t\tif (starts_with_mem(sig, len, gpg_format[i].sigs[j]))\n \t\t\t\treturn gpg_format + i;\n \treturn NULL;\n }\n \n const char *get_signature_format(const char *buf)\n {\n-\tstruct gpg_format *format = get_format_by_sig(buf);\n+\tstruct gpg_format *format = get_format_by_sig(buf, strlen(buf));\n \treturn format ? format->name : \"unknown\";\n }\n \n@@ -669,7 +669,7 @@ int check_signature(struct signature_check *sigc,\n \tsigc->result = 'N';\n \tsigc->trust_level = TRUST_UNDEFINED;\n \n-\tfmt = get_format_by_sig(signature);\n+\tfmt = get_format_by_sig(signature, slen);\n \tif (!fmt)\n \t\tdie(_(\"bad/incompatible signature '%s'\"), signature);\n \n@@ -706,7 +706,7 @@ size_t parse_signed_buffer(const char *buf, size_t size)\n \twhile (len < size) {\n \t\tconst char *eol;\n \n-\t\tif (get_format_by_sig(buf + len))\n+\t\tif (get_format_by_sig(buf + len, size - len))\n \t\t\tmatch = len;\n \n \t\teol = memchr(buf + len, '\\n', size - len);\n-- \ngitgitgadget\n\n"},{"id":"552822","messageId":"1cf4e5ddb5996423478b7543f7978e58cfcc4eb1.1789667556.git.gitgitgadget@gmail.com","threadId":"66341","inReplyTo":"pull.2231.git.1789667556.gitgitgadget@gmail.com","subject":"[PATCH 3/7] midx: validate incremental MIDX pack IDs","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-17T17:52:32Z","receivedAt":"2026-09-17T17:52:45Z","isPatch":true,"body":"From: Johannes Schindelin \n\nIncremental MIDX support made object-offset pack IDs local to each layer\nand then converted them to chain-global IDs by adding\n`num_packs_in_base`. The conversion was introduced by 19419821bac5\n(midx: teach `nth_midxed_pack_int_id()` about incremental MIDXs,\n2024-08-06). Chain-aware pack preparation followed in 1820bd878c62\n(midx: teach `prepare_midx_pack()` about incremental MIDXs, 2024-08-06),\nbut the final `midx_fill_entry()` lookup remained tied to the original\nlayer. Only with 8f909ff4e9e8 (packfile: recover when a multi-pack-index\nnames a removed pack, 2026-08-29) did Coverity point out this issue: a\nlocal ID such as `UINT32_MAX` could wrap when the base-pack count was\nadded, producing a plausible but incorrect global ID. After\n`prepare_midx_pack()` resolved the chain, `midx_fill_entry()` could then\nunderflow or address the wrong layer while indexing the current layer's\npack array, causing an invalid memory access and crashing Git.\n\nValidate each local pack ID against its layer's pack count before adding\nthe base count, and obtain the final pack through `nth_midxed_pack()`,\nwhich resolves the correct MIDX layer. This prevents an invalid local ID\nfrom wrapping during conversion and ensures that the lookup uses the\nlayer identified by the resolved chain-global ID.\n\nAssisted-by: GPT-5.6 Luna\nSigned-off-by: Johannes Schindelin \n---\n midx.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex 6d1c548e3d..6968fc1c00 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -583,10 +583,16 @@ off_t nth_midxed_offset(struct multi_pack_index *m, uint32_t pos)\n \n uint32_t nth_midxed_pack_int_id(struct multi_pack_index *m, uint32_t pos)\n {\n+\tuint32_t pack_int_id;\n+\n \tpos = midx_for_object(&m, pos);\n+\tpack_int_id = get_be32(m->chunk_object_offsets +\n+\t\t\t (off_t)pos * MIDX_CHUNK_OFFSET_WIDTH);\n+\tif (pack_int_id >= m->num_packs)\n+\t\tdie(_(\"bad pack-int-id: %\"PRIu32\" (%\"PRIu32\" total packs)\"),\n+\t\t pack_int_id, m->num_packs);\n \n-\treturn m->num_packs_in_base + get_be32(m->chunk_object_offsets +\n-\t\t\t\t\t (off_t)pos * MIDX_CHUNK_OFFSET_WIDTH);\n+\treturn m->num_packs_in_base + pack_int_id;\n }\n \n enum midx_fill_result midx_fill_entry(struct multi_pack_index *m,\n@@ -606,7 +612,7 @@ enum midx_fill_result midx_fill_entry(struct multi_pack_index *m,\n \n \tif (prepare_midx_pack(m, pack_int_id))\n \t\treturn MIDX_FILL_OWNER_UNAVAILABLE;\n-\tp = m->packs[pack_int_id - m->num_packs_in_base];\n+\tp = nth_midxed_pack(m, pack_int_id);\n \n \t/*\n \t* We are about to tell the caller where they can locate the\n-- \ngitgitgadget\n\n"},{"id":"552823","messageId":"bd9e06e46c6debb5a8fc8f1d3821250ca110d5ef.1789667556.git.gitgitgadget@gmail.com","threadId":"66341","inReplyTo":"pull.2231.git.1789667556.gitgitgadget@gmail.com","subject":"[PATCH 4/7] rerere: do not record failed conflict resolution data","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-17T17:52:33Z","receivedAt":"2026-09-17T17:52:46Z","isPatch":true,"body":"From: Johannes Schindelin \n\n`rerere` can mark a conflict variant as resolved even when writing its\npreimage or postimage fails. A later invocation may then replay\nincomplete data from the cache, turning a local filesystem failure into\nan incorrect working-tree change.\n\n629716d256a7 (rerere: do use multiple variants, 2015-07-30) introduced\nthe code paths without checks for those I/O results. Treat such failures\nas failures, report them, and leave the rerere status unchanged unless\nthe corresponding data was recorded successfully.\n\nThe defect has been latent since 2015. Git for Windows' Coverity run\nonly reported it after merging v2.56.0-rc0, for reasons that could not\nbe figured out in a reasonable amount of time.\n\nAssisted-by: GPT-5.6 Luna\nSigned-off-by: Johannes Schindelin \n---\n rerere.c | 30 ++++++++++++++++++++++++++----\n 1 file changed, 26 insertions(+), 4 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 1c3745d9e3..45bbe6ab2c 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -476,8 +476,11 @@ static int handle_file(struct index_state *istate,\n \t\t\tunlink_or_warn(output);\n \t\treturn error(_(\"could not parse conflict hunks in '%s'\"), path);\n \t}\n-\tif (io.io.wrerror)\n+\tif (io.io.wrerror) {\n+\t\tif (output)\n+\t\t\tunlink_or_warn(output);\n \t\treturn -1;\n+\t}\n \treturn has_conflicts;\n }\n \n@@ -729,8 +732,25 @@ static void do_rerere_one_path(struct index_state *istate,\n \n \t/* Has the user resolved it already? */\n \tif (variant >= 0) {\n-\t\tif (!handle_file(istate, path, NULL, NULL)) {\n-\t\t\tcopy_file(the_repository, rerere_path(&buf, id, \"postimage\"), path, 0666);\n+\t\tint ret = handle_file(istate, path, NULL, NULL);\n+\n+\t\tif (ret < 0)\n+\t\t\tgoto out;\n+\t\tif (!ret) {\n+\t\t\tconst int had_postimage =\n+\t\t\t\tid->collection->status[variant] & RR_HAS_POSTIMAGE;\n+\t\t\tconst char *postimage =\n+\t\t\t\trerere_path(&buf, id, \"postimage\");\n+\n+\t\t\tif (copy_file(the_repository,\n+\t\t\t\t postimage,\n+\t\t\t\t path, 0666)) {\n+\t\t\t\tif (!had_postimage)\n+\t\t\t\t\tunlink_or_warn(postimage);\n+\t\t\t\terror_errno(_(\"could not copy resolution for '%s'\"),\n+\t\t\t\t\t path);\n+\t\t\t\tgoto out;\n+\t\t\t}\n \t\t\tid->collection->status[variant] |= RR_HAS_POSTIMAGE;\n \t\t\tfprintf_ln(stderr, _(\"Recorded resolution for '%s'.\"), path);\n \t\t\tfree_rerere_id(rr_item);\n@@ -778,7 +798,9 @@ static void do_rerere_one_path(struct index_state *istate,\n \tassign_variant(id);\n \n \tvariant = id->variant;\n-\thandle_file(istate, path, NULL, rerere_path(&buf, id, \"preimage\"));\n+\tif (handle_file(istate, path, NULL,\n+\t\t\trerere_path(&buf, id, \"preimage\")) < 0)\n+\t\tgoto out;\n \tif (id->collection->status[variant] & RR_HAS_POSTIMAGE) {\n \t\tconst char *path = rerere_path(&buf, id, \"postimage\");\n \t\tif (unlink(path))\n-- \ngitgitgadget\n\n"},{"id":"552824","messageId":"bc67ad3b05a221fce939c8a4c6071769b949599b.1789667556.git.gitgitgadget@gmail.com","threadId":"66341","inReplyTo":"pull.2231.git.1789667556.gitgitgadget@gmail.com","subject":"[PATCH 5/7] t/unit-tests: check reftable iterator initialization","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-17T17:52:34Z","receivedAt":"2026-09-17T17:52:48Z","isPatch":true,"body":"From: Johannes Schindelin \n\nCoverity pointed out that the\n`test_reftable_table__seek_invalid_log_offset()` test, which was\nintroduced by a1c085df8dcb (reftable/table: fix NULL pointer access when\nseeking to bogus offsets, 2026-07-03), ignores the result of\n`reftable_table_init_log_iterator()` and proceeds to\n`reftable_iterator_seek_log()`, although initialization can return\n`REFTABLE_OUT_OF_MEMORY_ERROR` without installing an ops table. Under\nallocation failure, the test then dereferences a NULL function table.\n\nAssert successful iterator initialization before seeking.\n\nAssisted-by: GPT-5.6 Luna\nSigned-off-by: Johannes Schindelin \n---\n t/unit-tests/u-reftable-table.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/unit-tests/u-reftable-table.c b/t/unit-tests/u-reftable-table.c\nindex bd04b477a3..1e4378b2eb 100644\n--- a/t/unit-tests/u-reftable-table.c\n+++ b/t/unit-tests/u-reftable-table.c\n@@ -257,7 +257,7 @@ void test_reftable_table__seek_invalid_log_offset(void)\n \t * know that the table is corrupt, so the seek must report a format\n \t * error instead of pretending that the section is empty.\n \t */\n-\treftable_table_init_log_iterator(table, &it);\n+\tcl_assert_equal_i(reftable_table_init_log_iterator(table, &it), 0);\n \tcl_assert_equal_i(reftable_iterator_seek_log(&it, \"\"),\n \t\t\t REFTABLE_FORMAT_ERROR);\n \n-- \ngitgitgadget\n\n"},{"id":"552825","messageId":"de28e72d5f5e3fe59666919510242daf82845337.1789667556.git.gitgitgadget@gmail.com","threadId":"66341","inReplyTo":"pull.2231.git.1789667556.gitgitgadget@gmail.com","subject":"[PATCH 6/7] oss-fuzz: handle reftable iterator initialization failures","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-17T17:52:35Z","receivedAt":"2026-09-17T17:52:50Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe reftable fuzzer introduced by adf45165e65b (oss-fuzz: add fuzzer for\nparsing reftables, 2026-07-03) ignored failures from\n`reftable_table_init_ref_iterator()` and\n`reftable_table_init_log_iterator()`. Coverity reported that under\nallocation failure, either constructor can return\n`REFTABLE_OUT_OF_MEMORY_ERROR` without installing an ops table, allowing\na subsequent seek to dereference NULL.\n\nTreat iterator initialization failure as a reason to skip the\ncorresponding seek and iteration while retaining safe destruction for an\nuninitialized iterator.\n\nAssisted-by: GPT-5.6 Luna\nSigned-off-by: Johannes Schindelin \n---\n oss-fuzz/fuzz-reftable.c | 18 ++++++++++--------\n 1 file changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/oss-fuzz/fuzz-reftable.c b/oss-fuzz/fuzz-reftable.c\nindex c46eac2c6b..75b8ad0c3d 100644\n--- a/oss-fuzz/fuzz-reftable.c\n+++ b/oss-fuzz/fuzz-reftable.c\n@@ -33,10 +33,11 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)\n \t\tstruct reftable_ref_record ref = { 0 };\n \t\tstruct reftable_iterator it = { 0 };\n \n-\t\treftable_table_init_ref_iterator(table, &it);\n-\t\tif (!reftable_iterator_seek_ref(&it, \"\"))\n-\t\t\twhile (!reftable_iterator_next_ref(&it, &ref))\n-\t\t\t\t;\n+\t\tif (!reftable_table_init_ref_iterator(table, &it)) {\n+\t\t\tif (!reftable_iterator_seek_ref(&it, \"\"))\n+\t\t\t\twhile (!reftable_iterator_next_ref(&it, &ref))\n+\t\t\t\t\t;\n+\t\t}\n \n \t\treftable_ref_record_release(&ref);\n \t\treftable_iterator_destroy(&it);\n@@ -46,10 +47,11 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)\n \t\tstruct reftable_log_record log = { 0 };\n \t\tstruct reftable_iterator it = { 0 };\n \n-\t\treftable_table_init_log_iterator(table, &it);\n-\t\tif (!reftable_iterator_seek_log(&it, \"\"))\n-\t\t\twhile (!reftable_iterator_next_log(&it, &log))\n-\t\t\t\t;\n+\t\tif (!reftable_table_init_log_iterator(table, &it)) {\n+\t\t\tif (!reftable_iterator_seek_log(&it, \"\"))\n+\t\t\t\twhile (!reftable_iterator_next_log(&it, &log))\n+\t\t\t\t\t;\n+\t\t}\n \n \t\treftable_log_record_release(&log);\n \t\treftable_iterator_destroy(&it);\n-- \ngitgitgadget\n\n"},{"id":"552826","messageId":"60599d24542722b1393483a0b0ff6eff2325ad36.1789667556.git.gitgitgadget@gmail.com","threadId":"66341","inReplyTo":"pull.2231.git.1789667556.gitgitgadget@gmail.com","subject":"[PATCH 7/7] test-read-midx: check midx_fill_entry() result","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-17T17:52:36Z","receivedAt":"2026-09-17T17:52:53Z","isPatch":true,"body":"From: Johannes Schindelin \n\nThe `--show-objects` mode of `read_midx_file()` uses the output of\n`midx_fill_entry()` without checking whether the lookup succeeded. A\nfailed lookup or unavailable pack can leave that output unusable,\nallowing malformed or concurrently changed MIDX data to make this test\nhelper crash instead of reporting a controlled error.\n\nReject the entry unless `midx_fill_entry()` returns `MIDX_FILL_HIT`. The\nunchecked call was introduced by 86d174b7246b\n(t/helper/test-read-midx.c: add '--show-objects', 2021-03-30); later\nincremental-MIDX changes expanded the possible failure modes, but this\nremains a test-helper robustness issue, not a production Git attack\nsurface or an arbitrary-code-execution vulnerability.\n\nIt is unclear why Coverity reports this issue in Git for Windows only\nafter merging v2.56.0-rc0; The issue was not reported before.\n\nAssisted-by: GPT-5.6 Luna\nSigned-off-by: Johannes Schindelin \n---\n t/helper/test-read-midx.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c\nindex 83b07c6236..412089563f 100644\n--- a/t/helper/test-read-midx.c\n+++ b/t/helper/test-read-midx.c\n@@ -90,7 +90,11 @@ static int read_midx_file(const char *object_dir, const char *checksum,\n \t\tfor (i = 0; i < m->num_objects; i++) {\n \t\t\tnth_midxed_object_oid(&oid, m,\n \t\t\t\t\t i + m->num_objects_in_base);\n-\t\t\tmidx_fill_entry(m, &oid, &e, NULL);\n+\t\t\tif (midx_fill_entry(m, &oid, &e, NULL) !=\n+\t\t\t MIDX_FILL_HIT) {\n+\t\t\t\tret = error(_(\"failed to load pack entry\"));\n+\t\t\t\tgoto out;\n+\t\t\t}\n \n \t\t\tprintf(\"%s %\"PRIu64\"\\t%s\\n\",\n \t\t\t oid_to_hex(&oid), e.offset, e.p->pack_name);\n-- \ngitgitgadget\n"},{"id":"552832","messageId":"xmqqa4pfu11i.fsf@gitster.g","threadId":"66341","inReplyTo":"3fc7774ba867a10f35f7a74424baa4527f038232.1789667556.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/7] gpg-interface: make signature-prefix matching length-aware","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-17T19:32:09Z","receivedAt":"2026-09-17T19:32:11Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> @@ -669,7 +669,7 @@ int check_signature(struct signature_check *sigc,\n> \tsigc->result = 'N';\n> \tsigc->trust_level = TRUST_UNDEFINED;\n> \n> -\tfmt = get_format_by_sig(signature);\n> +\tfmt = get_format_by_sig(signature, slen);\n> \tif (!fmt)\n> \t\tdie(_(\"bad/incompatible signature '%s'\"), signature);\n\nAll the existing callers of check_signature() pass a NUL-terminated\nbuffer which is pair of a strbuf. Another approach that\nmay be simpler is to drop the slen parameter from check_signature().\n\n"},{"id":"552833","messageId":"xmqq5x03u0xz.fsf@gitster.g","threadId":"66341","inReplyTo":"bd9e06e46c6debb5a8fc8f1d3821250ca110d5ef.1789667556.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/7] rerere: do not record failed conflict resolution data","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-17T19:34:16Z","receivedAt":"2026-09-17T19:34:18Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> @@ -778,7 +798,9 @@ static void do_rerere_one_path(struct index_state *istate,\n> \tassign_variant(id);\n> \n> \tvariant = id->variant;\n> -\thandle_file(istate, path, NULL, rerere_path(&buf, id, \"preimage\"));\n> +\tif (handle_file(istate, path, NULL,\n> +\t\t\trerere_path(&buf, id, \"preimage\")) < 0)\n> +\t\tgoto out;\n> \tif (id->collection->status[variant] & RR_HAS_POSTIMAGE) {\n> \t\tconst char *path = rerere_path(&buf, id, \"postimage\");\n> \t\tif (unlink(path))\n\nGood to see this one, which is the only unchecked call to the\nhandle_file() function, checked for an error. Looking good.\n\nThanks.\n"},{"id":"552850","messageId":"e20a33ef-181c-6dae-ce9a-8dfbc5d560b0@gmx.de","threadId":"66341","inReplyTo":"xmqqa4pfu11i.fsf@gitster.g","subject":"Re: [PATCH 2/7] gpg-interface: make signature-prefix matching length-aware","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-09-18T07:12:32Z","receivedAt":"2026-09-18T07:12:37Z","isPatch":true,"body":"Hi Junio,\n\nOn Thu, 17 Sep 2026, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > @@ -669,7 +669,7 @@ int check_signature(struct signature_check *sigc,\n> > \tsigc->result = 'N';\n> > \tsigc->trust_level = TRUST_UNDEFINED;\n> > \n> > -\tfmt = get_format_by_sig(signature);\n> > +\tfmt = get_format_by_sig(signature, slen);\n\nThis hunk is a direct consequence of `get_format_by_sig()` gaining a\nlength parameter earlier in the same patch: it now has three callers,\n`get_signature_format()`, `check_signature()` here, and the loop inside\n`parse_signed_buffer()` (the actual site of the bug this series fixes,\nCoverity issue with CID 1678690 if you want to double-check).\n\nOnce the function takes a (sig, len) pair uniformly, every one of them has\nto pass a length, so this call site is not optional scaffolding; it is\nwhat makes all three callers correct by construction instead of leaving\ntwo of them trusting NUL-termination and one bounds-checked.\n\n> > \tif (!fmt)\n> > \t\tdie(_(\"bad/incompatible signature '%s'\"), signature);\n> \n> All the existing callers of check_signature() pass a NUL-terminated\n> buffer which is pair of a strbuf.\n\nThat holds for six of the seven call sites: `commit.c`, `tag.c`,\n`builtin/fast-import.c`, `fmt-merge-msg.c`, and `log-tree.c` (twice).\n`builtin/receive-pack.c` is a minor wrinkle worth flagging: it passes\n`push_cert.buf + bogs` (\"bogs\" = \"beginning_of_gpg_sig\") and\n`push_cert.len - bogs`, an offset sub-buffer of `push_cert`, not that\nstrbuf's own buf/len pair verbatim. It still ends on `push_cert`'s own\nterminating NUL, so the observation holds in spirit, but strictly the\npattern is \"ends at some strbuf's own NUL\", which is more a matter of\ncode-review convention across call sites than something\n`check_signature()`'s own signature guarantees.\n\n> Another approach that may be simpler is to drop the slen parameter from\n> check_signature().\n\nI would rather keep it right where it is, for (at least 😊) two reasons.\n\nFirst, `slen` isn't new here: `check_signature()` has taken a `(sigc,\nsignature, slen)` signature since 02769437e142 (ssh signing: use sigc\nstruct to pass payload, 2021-12-09), three years before this series, so\ndropping it now would fold an unrelated API change into a bug fix.\n\nSecond, and this is the one that actually worries me: `slen` is used twice\ninside `check_signature()`, not once. Besides the `get_format_by_sig()`\ncall above, the pre-existing `fmt->verify_signed_buffer(sigc, fmt,\nsignature, slen)` a few lines down depends on it too (there it is named\n`signature_size`). Both concrete implementations of that vtable member,\n`verify_gpg_signed_buffer()` and `verify_ssh_signed_buffer()`, use\n`signature_size` to decide exactly how many bytes to `write_in_full()`\ninto the temporary file that then gets handed to `gpg`/`ssh-keygen` as the\ndetached signature to verify. That is the authoritative byte count of the\nblob being verified, not a defensive nicety. If we dropped `slen` and let\n`check_signature()` fall back on `strlen(signature)`, a signature blob\nwith an embedded NUL before its logical end would get truncated before it\never reaches the external verifier: a correctness regression in the actual\ncryptographic verification path, not merely in the prefix-matching helper\nthis series fixes. `check_signature()` has no doc comment promising\n`signature` is free of embedded NULs, so dropping `slen` would trade an\nexplicit length for an implicit assumption.\n\nAs the commit message notes, we have been down this road with this exact\nfunction chain before. In February 2024, Peff concluded there was no\nwalk-too-far problem in `parse_signed_buffer()` \"because we feed it from a\nstrbuf\":\nhttps://lore.kernel.org/git/20240208214137.GB1090198@coredump.intra.peff.net/\n\nBut that conclusion was already four months stale: c8762c30df5b\n(object-file-convert: convert tag objects when writing, 2023-10-01) had\nalready added `convert_tag_object()` as a caller that does _not_ feed from\na strbuf: the same gap this series closes. Applying the same \"audit\ntoday's callers and assume it holds\" reasoning to `check_signature()` now\nrisks reproducing that failure mode a second time.\n\nSo I would like to keep `slen` and the `get_format_by_sig(signature,\nslen)` call as in the patch.\n\nCiao,\nJohannes\n"},{"id":"552854","messageId":"xmqq1parorz4.fsf@gitster.g","threadId":"66341","inReplyTo":"e20a33ef-181c-6dae-ce9a-8dfbc5d560b0@gmx.de","subject":"Re: [PATCH 2/7] gpg-interface: make signature-prefix matching length-aware","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-18T08:59:11Z","receivedAt":"2026-09-18T08:59:14Z","isPatch":true,"body":"Johannes Schindelin writes:\n\n> That holds for six of the seven call sites: `commit.c`, `tag.c`,\n> `builtin/fast-import.c`, `fmt-merge-msg.c`, and `log-tree.c` (twice).\n> `builtin/receive-pack.c` is a minor wrinkle worth flagging: it passes\n> `push_cert.buf + bogs` (\"bogs\" = \"beginning_of_gpg_sig\") and\n> `push_cert.len - bogs`, an offset sub-buffer of `push_cert`, not that\n> strbuf's own buf/len pair verbatim. It still ends on `push_cert`'s own\n> terminating NUL, so the observation holds in spirit, but strictly the\n> pattern is \"ends at some strbuf's own NUL\", which is more a matter of\n> code-review convention across call sites than something\n> `check_signature()`'s own signature guarantees.\n\nYes but the audit was \"is slen our callers pass redundant?\", and not\n\"does everybody pass strbuf and we are better off passing a pionter\nto a strbuf?\". And the answer to the former question is \"yes\".\n\nAnd I do not quite understand or agree with the logic here.\n\n> ... Applying the same \"audit\n> today's callers and assume it holds\" reasoning to `check_signature()` now\n> risks reproducing that failure mode a second time.\n\nWhat I was saying was to force all current *and* *future* callers to\npass NUL-terminated string by removing slen.\n\nHaving said all that, I think this falls into \"once the code is\nwritten (and more importantly, once it is reviewed, as that is a lot\nmore costly part of the development process for machine written\ncode), it is not worth going back and change it, as the difference\nis not large enough either way.\"\n"}]}