From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 2/7] gpg-interface: make signature-prefix matching length-aware
Date: Thu, 17 Sep 2026 17:52:31 +0000 [thread overview]
Message-ID: <3fc7774ba867a10f35f7a74424baa4527f038232.1789667556.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2231.git.1789667556.gitgitgadget@gmail.com>
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
next prev parent reply other threads:[~2026-09-17 17:52 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-17 17:52 [PATCH 0/7] Fix issues pointed out in Git for Windows by Coverity after merging v2.56.0-rc0 Johannes Schindelin via GitGitGadget
2026-09-17 17:52 ` [PATCH 1/7] wrapper: guard writev_in_full() against signed overflow Johannes Schindelin via GitGitGadget
2026-09-17 17:52 ` Johannes Schindelin via GitGitGadget [this message]
2026-09-17 19:32 ` [PATCH 2/7] gpg-interface: make signature-prefix matching length-aware Junio C Hamano
2026-09-18 7:12 ` Johannes Schindelin
2026-09-18 8:59 ` Junio C Hamano
2026-09-17 17:52 ` [PATCH 3/7] midx: validate incremental MIDX pack IDs Johannes Schindelin via GitGitGadget
2026-09-17 17:52 ` [PATCH 4/7] rerere: do not record failed conflict resolution data Johannes Schindelin via GitGitGadget
2026-09-17 19:34 ` Junio C Hamano
2026-09-17 17:52 ` [PATCH 5/7] t/unit-tests: check reftable iterator initialization Johannes Schindelin via GitGitGadget
2026-09-17 17:52 ` [PATCH 6/7] oss-fuzz: handle reftable iterator initialization failures Johannes Schindelin via GitGitGadget
2026-09-17 17:52 ` [PATCH 7/7] test-read-midx: check midx_fill_entry() result Johannes Schindelin via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3fc7774ba867a10f35f7a74424baa4527f038232.1789667556.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox