shorten_unambiguous_ref(): avoid integer truncation
commitdd5e4d39765f44492031f2d9319c24f0868bbe1a
authorJeff King <peff@peff.net>
Wed, 15 Feb 2023 15:16:14 +0000 (15 10:16 -0500)
committerJunio C Hamano <gitster@pobox.com>
Wed, 15 Feb 2023 16:53:17 +0000 (15 08:53 -0800)
tree0b6bfde325edd9944b61f34ec617e818355d3f24
parentcbf04937d5b9fcf0a76c28f69e6294e9e3ecd7e6
shorten_unambiguous_ref(): avoid integer truncation

We parse the shortened name "foo" out of the full refname
"refs/heads/foo", and then assign the result of strlen(short_name) to an
int, which may truncate or wrap to negative.

In practice, this should never happen, as it requires a 2GB refname. And
even somebody trying to do something malicious should at worst end up
with a confused answer (we use the size only to feed back as a
placeholder length to strbuf_addf() to see if there are any collisions
in the lookup rules).

And it may even be impossible to trigger this, as we parse the string
with sscanf(), and stdio formatting functions are not known for handling
large strings well. I didn't test, but I wouldn't be surprised if
sscanf() on many platforms simply reports no match here.

But even if it is not a problem in practice so far, it is worth fixing
for two reasons:

  1. We'll shortly be replacing the sscanf() call with a real parser
     which will handle arbitrary-sized strings.

  2. Assigning strlen() to an int is an anti-pattern that requires
     people to look twice when auditing for real overflow problems.

So we'll make this a size_t. Unfortunately we still have to cast to int
eventually for the strbuf_addf() call, but at least we can localize the
cast there, and check that it will be valid. I used our new cast helper
here, which will just bail completely. That should be OK, as anybody
with a 2GB refname is up to no good, but if we really wanted to, we
could detect it manually and just refuse to shorten the refname.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs.c