Re: git segfault in tag verify (patch included)

[Junio C Hamano <gitster@pobox.com>]

Steven Roberts <fenderq@gmail.com> writes:

> I believe I have found an off-by-one error in git.
>
> Please see https://marc.info/?l=openbsd-ports&m=156326783610123&w=2

That is this thing.

        static void parse_gpg_output(struct signature_check *sigc)
        {
                const char *buf = sigc->gpg_status;
                const char *line, *next;
                int i, j;
                int seen_exclusive_status = 0;

                /* Iterate over all lines */
                for (line = buf; *line; line = strchrnul(line+1, '\n')) {
                        while (*line == '\n')
                                line++;
                        /* Skip lines that don't start with GNUPG status */
                        if (!skip_prefix(line, "[GNUPG:] ", &line))
                                continue;

If the GPG output ends with a trailing blank line, we skip and get
to the terminating NUL, then find that it does not begin with
the "[GNUPG:] " prefix, and hit the continue.  We try to scan and
look for LF (or stop at the end of the string) for the next round,
starting at one past where we are, which is already the terminating
NUL.  Ouch.

Good finding.

We need your sign-off (see Documentation/SubmittingPatches).

Thanks.


-- >8 --
From: Steven Roberts <fenderq@gmail.com>
Subject: gpg-interface: do not scan past the end of buffer

If the GPG output ends with trailing blank lines, after skipping
them over inside the loop to find the terminating NUL at the end,
the loop ends up looking for the next line, starting past the end.

---
 gpg-interface.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gpg-interface.c b/gpg-interface.c
index 8ed274533f..eb55d46ea4 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -116,6 +116,9 @@ static void parse_gpg_output(struct signature_check *sigc)
 	for (line = buf; *line; line = strchrnul(line+1, '\n')) {
 		while (*line == '\n')
 			line++;
+		if (!*line)
+			break;
+
 		/* Skip lines that don't start with GNUPG status */
 		if (!skip_prefix(line, "[GNUPG:] ", &line))
 			continue;