Re: [PATCH v2 6/9] gpg-interface: do not hardcode the key string len anymore
- From
Jeff King <peff@peff.net>
- Date
- Jul 11, 2018, 14:27 UTC
- Message-ID
- <20180711142751.GF23835@sigill.intra.peff.net>
- In-Reply-To
- <20180711154619.7b574dff@md1pvb1c.ad001.siemens.net>
On Wed, Jul 11, 2018 at 03:46:19PM +0200, Henning Schild wrote:
Show 10 quoted lines
> > I think it's worth addressing in the near term, if only because this > > kind of off-by-one is quite subtle, and I don't want to forget to deal > > with it. Whether that happens as part of this patch, or as a cleanup > > before or after, I'm not picky. :) > > I get that and if anyone is willing to write that code, i will base my > patches on it. What i want to avoid is taking responsibility for > problems i did not introduce, just because i happen to work on that > code at the moment. Keeping track of that (not forgetting) is also not > for the random contributor like myself.
It doesn't make sense to do a patch before your series, since it would just be:
if (strlen(found) > 16)
...which would get obliterated by your patch. The patch after is shown below. But frankly, it seems a lot easier to just handle this while you are rewriting the code.
-- >8 -- Subject: [PATCH] gpg-interface: handle off-by-one parsing gpg output
When parsing gpg's VALIDSIG lines, we look for a space followed by the signer information. Because we use strchrnul(), though, if the space is missing we'll end up pointing to the trailing NUL. When we try to move past that space, we have to handle the NUL case separately to avoid accidentally stepping out of the string entirely.
Signed-off-by: Jeff King <peff@peff.net> --- gpg-interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gpg-interface.c b/gpg-interface.c index bf8d567a4c..139b0f561e 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -97,7 +97,7 @@ static void parse_gpg_output(struct signature_check *sigc) sigc->key = xmemdupz(found, next - found); /* The ERRSIG message is not followed by signer information */ if (sigc-> result != 'E') { - found = next + 1; + found = *next ? next + 1 : next; next = strchrnul(found, '\n'); sigc->signer = xmemdupz(found, next - found); }
-- 2.18.0.400.g702e398724