don't accept bogus N in `HEAD~N'
- From
Jim Meyering <jim@meyering.net>
- Date
- May 21, 2006, 21:35 UTC
- Message-ID
- <87d5e7hxhl.fsf_-_@rho.meyering.net>
- In-Reply-To
- <7v3bf3jl15.fsf@assigned-by-dhcp.cox.net>
In a very shallow audit, I spotted code where overflow was not detected. But it's hardly critical.
Currently,
git-diff HEAD HEAD
is equivalent to this
git-diff HEAD HEAD~18446744073709551616 # aka 2^64
Exercising git-rev-parse directly, currently I get this:
$ git-rev-parse --no-flags --sq HEAD~18446744073709551616 '639ca5497279607665847f2e3a11064441a8f2a6'
It'd be better to produce a diagnostic and fail:
$ ./git-rev-parse --no-flags --sq -- HEAD~18446744073709551616 > /dev/null fatal: ambiguous argument 'HEAD~18446744073709551616': unknown revision or filename
The code in question is in sha1_name.c (get_sha1_1):
int num = 0;
...
while (cp < name + len)
num = num * 10 + *cp++ - '0';Looking at how to fix it, my first reflex was to replace that loop with this one:
while (cp < name + len) {
int tmp = num * 10 + *cp++ - '0';
if (INT_MAX / 10 < num || tmp < num)
return -1;
num = tmp;
}But INT_MAX is used nowhere else, so I wonder if git avoids using it for some reason. At least `make check' gripes about __INT_MAX__. Anyhow, here's the patch I used. With it, git still passes `make test'.
diff --git a/sha1_name.c b/sha1_name.c index dc68355..c813ba0 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -429,8 +429,12 @@ static int get_sha1_1(const char *name, int num = 0; int len1 = cp - name; cp++; - while (cp < name + len) - num = num * 10 + *cp++ - '0'; + while (cp < name + len) { + int tmp = num * 10 + *cp++ - '0'; + if (INT_MAX / 10 < num || tmp < num) + return -1; + num = tmp; + } if (has_suffix == '^') { if (!num && len1 == len - 1) num = 1;