Re: [PATCH 1/2] gpg-interface: handle missing " with " gracefully in parse_ssh_output()

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

René Scharfe <l.s.r@web.de> writes:

> If the output of ssh-keygen starts with "Good \"git\" signature for ",
> but is not followed by " with " for some reason, then parse_ssh_output()
> uses -1 as the len parameter of xmemdupz(), which in turn will end the
> program.  Reject the signature and carry on instead in that case.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> This code was added after v2.33.0.
> Patch formatted with --inter-hunk-context=2 for easier review.

Nice spotting.

> Silly bonus question: What's the purpose of the "+ 1" and "- 1", which
> seem to cancel each other out?

Fabian, we are in -rc phase where we concentrate on fixing bugs in
the new code in 'master'.  A quick ack, "here is a better way to fix
it", or "no, that won't be needed because..." is very much
appreciated.

Thanks.

>
>  gpg-interface.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 800d8caa67..62d340e78a 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -387,17 +387,19 @@ static void parse_ssh_output(struct signature_check *sigc)
>  	line = to_free = xmemdupz(sigc->output, strcspn(sigc->output, "\n"));
>
>  	if (skip_prefix(line, "Good \"git\" signature for ", &line)) {
> -		/* Valid signature and known principal */
> -		sigc->result = 'G';
> -		sigc->trust_level = TRUST_FULLY;
> -
>  		/* Search for the last "with" to get the full principal */
>  		principal = line;
>  		do {
>  			search = strstr(line, " with ");
>  			if (search)
>  				line = search + 1;
>  		} while (search != NULL);
> +		if (line == principal)
> +			goto cleanup;
> +
> +		/* Valid signature and known principal */
> +		sigc->result = 'G';
> +		sigc->trust_level = TRUST_FULLY;
>  		sigc->signer = xmemdupz(principal, line - principal - 1);
>  	} else if (skip_prefix(line, "Good \"git\" signature with ", &line)) {
>  		/* Valid signature, but key unknown */
> --
> 2.33.1