Skip to content

Commit dd5e4d3

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent cbf0493 commit dd5e4d3

File tree

1 file changed

+3
-2
lines changed

1 file changed

+3
-2
lines changed

refs.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,7 +1356,7 @@ char *refs_shorten_unambiguous_ref(struct ref_store *refs,
13561356
for (i = nr_rules - 1; i > 0 ; --i) {
13571357
int j;
13581358
int rules_to_fail = i;
1359-
int short_name_len;
1359+
size_t short_name_len;
13601360

13611361
if (1 != sscanf(refname, scanf_fmts[i], short_name))
13621362
continue;
@@ -1388,7 +1388,8 @@ char *refs_shorten_unambiguous_ref(struct ref_store *refs,
13881388
*/
13891389
strbuf_reset(&resolved_buf);
13901390
strbuf_addf(&resolved_buf, rule,
1391-
short_name_len, short_name);
1391+
cast_size_t_to_int(short_name_len),
1392+
short_name);
13921393
if (refs_ref_exists(refs, resolved_buf.buf))
13931394
break;
13941395
}

0 commit comments

Comments
 (0)