On 2019-09-15 at 12:10:28, René Scharfe wrote: > Reject values that don't fit into an int, as get_parent() and > get_nth_ancestor() cannot handle them. That's better than potentially > returning a random object. > > If this restriction turns out to be too tight then we can switch to a > wider data type, but we'd still have to check for overflow.
Certainly we want Git to perform as well as possible on large
repositories, but I doubt if it will scale to more than 2 billion
revisions, even with significant effort. I think this restriction
should be fine.
> diff --git a/sha1-name.c b/sha1-name.c
> index c665e3f96d..7a047e9e2b 100644
> --- a/sha1-name.c
> +++ b/sha1-name.c
> @@ -1160,13 +1160,22 @@ static enum get_oid_result get_oid_1(struct
> repository *r,
> }
>
> if (has_suffix) {
> - int num = 0;
> + unsigned int num = 0;
> int len1 = cp - name;
> cp++;
> - while (cp < name + len)
> - num = num * 10 + *cp++ - '0';
> + while (cp < name + len) {
> + unsigned int digit = *cp++ - '0';
> + if (unsigned_mult_overflows(num, 10))
> + return MISSING_OBJECT;
> + num *= 10;
> + if (unsigned_add_overflows(num, digit))
> + return MISSING_OBJECT;
I was worried whether these functions only handled size_t or if they
also handle unsigned int, but I checked and they seem to be fine for any
unsigned type.
> + num += digit;
> + }
> if (!num && len1 == len - 1)
> num = 1;
> + else if (num > INT_MAX)
> + return MISSING_OBJECT;
> if (has_suffix == '^')
> return get_parent(r, name, len1, oid, num);
> /* else if (has_suffix == '~') -- goes without saying */
This approach seems reasonable. I must admit some curiosity as to how
you discovered this issue, though. Did you have a cat assisting you in
typing revisions?
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
signature.asc
Description: PGP signature
