Re: [PATCH 2/2] sha1-name: check for overflow of N in "foo^N" and "foo~N"
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Sep 15, 2019, 15:15 UTC
- Message-ID
- <20190915151513.GU11334@genre.crustytoothpaste.net>
- In-Reply-To
- <2be6e3ee-209e-9cd1-eb43-284f9a8462b3@web.de>
On 2019-09-15 at 12:10:28, René Scharfe wrote:
Show 6 quoted lines
> 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.
Show 21 quoted lines
> 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.
Show 9 quoted lines
> + 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