[tor-commits] [tor/master] Avoid overflows and underflows in sscanf and friends
nickm at torproject.org
nickm at torproject.org
Thu Sep 11 04:15:42 UTC 2014
commit d2463c0cfee066111c3a72d188cd897957aa2988
Author: Nick Mathewson <nickm at torproject.org>
Date: Wed Sep 10 23:57:31 2014 -0400
Avoid overflows and underflows in sscanf and friends
(Patch from teor on 13104)
---
src/common/util.c | 23 +++++--
src/test/test_util.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 171 insertions(+), 19 deletions(-)
diff --git a/src/common/util.c b/src/common/util.c
index 2d7893b..297ae78 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -2804,10 +2804,14 @@ scan_unsigned(const char **bufp, unsigned long *out, int width, int base)
while (**bufp && (hex?TOR_ISXDIGIT(**bufp):TOR_ISDIGIT(**bufp))
&& scanned_so_far < width) {
int digit = hex?hex_decode_digit(*(*bufp)++):digit_to_num(*(*bufp)++);
- unsigned long new_result = result * base + digit;
- if (new_result < result)
- return -1; /* over/underflow. */
- result = new_result;
+ // Check for overflow beforehand, without actually causing any overflow
+ // This preserves functionality on compilers that don't wrap overflow
+ // (i.e. that trap or optimise away overflow)
+ // result * base + digit > ULONG_MAX
+ // result * base > ULONG_MAX - digit
+ if (result > (ULONG_MAX - digit)/base)
+ return -1; /* Processing this digit would overflow */
+ result = result * base + digit;
++scanned_so_far;
}
@@ -2842,10 +2846,17 @@ scan_signed(const char **bufp, long *out, int width)
if (scan_unsigned(bufp, &result, width, 10) < 0)
return -1;
- if (neg) {
+ if (neg && result > 0) {
if (result > ((unsigned long)LONG_MAX) + 1)
return -1; /* Underflow */
- *out = -(long)result;
+ // Avoid overflow on the cast to signed long when result is LONG_MIN
+ // by subtracting 1 from the unsigned long positive value,
+ // then, after it has been cast to signed and negated,
+ // subtracting the original 1 (the double-subtraction is intentional).
+ // Otherwise, the cast to signed could cause a temporary long
+ // to equal LONG_MAX + 1, which is undefined.
+ // We avoid underflow on the subtraction by treating -0 as positive.
+ *out = (-(long)(result - 1)) - 1;
} else {
if (result > LONG_MAX)
return -1; /* Overflow */
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 151ec69..16a2927 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -1672,6 +1672,7 @@ static void
test_util_sscanf(void)
{
unsigned u1, u2, u3;
+ unsigned long ulng;
char s1[20], s2[10], s3[10], ch;
int r;
long lng1,lng2;
@@ -1713,11 +1714,6 @@ test_util_sscanf(void)
test_eq(0, tor_sscanf("", "%u", &u1)); /* absent number */
test_eq(0, tor_sscanf("A", "%u", &u1)); /* bogus number */
test_eq(0, tor_sscanf("-1", "%u", &u1)); /* negative number */
- test_eq(1, tor_sscanf("4294967295", "%u", &u1)); /* UINT32_MAX should work */
- test_eq(4294967295u, u1);
- test_eq(0, tor_sscanf("4294967296", "%u", &u1)); /* But not at 32 bits */
- test_eq(1, tor_sscanf("4294967296", "%9u", &u1)); /* but parsing only 9... */
- test_eq(429496729u, u1);
/* Numbers with size (eg. %2u) */
test_eq(0, tor_sscanf("-1", "%2u", &u1));
@@ -1812,46 +1808,191 @@ test_util_sscanf(void)
test_eq(int2, -1);
#if SIZEOF_INT == 4
+ /* %u */
+ /* UINT32_MAX should work */
+ test_eq(1, tor_sscanf("4294967295", "%u", &u1));
+ test_eq(4294967295U, u1);
+
+ /* But UINT32_MAX + 1 shouldn't work */
+ test_eq(0, tor_sscanf("4294967296", "%u", &u1));
+ /* but parsing only 9... */
+ test_eq(1, tor_sscanf("4294967296", "%9u", &u1));
+ test_eq(429496729U, u1);
+
+ /* %x */
+ /* UINT32_MAX should work */
+ test_eq(1, tor_sscanf("FFFFFFFF", "%x", &u1));
+ test_eq(0xFFFFFFFF, u1);
+
+ /* But UINT32_MAX + 1 shouldn't work */
+ test_eq(0, tor_sscanf("100000000", "%x", &u1));
+
+ /* %d */
+ /* INT32_MIN and INT32_MAX should work */
r = tor_sscanf("-2147483648. 2147483647.", "%d. %d.", &int1, &int2);
test_eq(r,2);
- test_eq(int1, -2147483647-1);
+ test_eq(int1, -2147483647 - 1);
test_eq(int2, 2147483647);
- r = tor_sscanf("-2147483679.", "%d.", &int1);
+ /* But INT32_MIN - 1 and INT32_MAX + 1 shouldn't work */
+ r = tor_sscanf("-2147483649.", "%d.", &int1);
test_eq(r,0);
- r = tor_sscanf("2147483678.", "%d.", &int1);
+ r = tor_sscanf("2147483648.", "%d.", &int1);
+ test_eq(r,0);
+
+ /* and the first failure stops further processing */
+ r = tor_sscanf("-2147483648. 2147483648.",
+ "%d. %d.", &int1, &int2);
+ test_eq(r,1);
+
+ r = tor_sscanf("-2147483649. 2147483647.",
+ "%d. %d.", &int1, &int2);
+ test_eq(r,0);
+
+ r = tor_sscanf("2147483648. -2147483649.",
+ "%d. %d.", &int1, &int2);
test_eq(r,0);
#elif SIZEOF_INT == 8
+ /* %u */
+ /* UINT64_MAX should work */
+ test_eq(1, tor_sscanf("18446744073709551615", "%u", &u1));
+ test_eq(18446744073709551615U, u1);
+
+ /* But UINT64_MAX + 1 shouldn't work */
+ test_eq(0, tor_sscanf("18446744073709551616", "%u", &u1));
+ /* but parsing only 19... */
+ test_eq(1, tor_sscanf("18446744073709551616", "%19u", &u1));
+ test_eq(1844674407370955161U, u1);
+
+ /* %x */
+ /* UINT64_MAX should work */
+ test_eq(1, tor_sscanf("FFFFFFFFFFFFFFFF", "%x", &u1));
+ test_eq(0xFFFFFFFFFFFFFFFF, u1);
+
+ /* But UINT64_MAX + 1 shouldn't work */
+ test_eq(0, tor_sscanf("10000000000000000", "%x", &u1));
+
+ /* %d */
+ /* INT64_MIN and INT64_MAX should work */
r = tor_sscanf("-9223372036854775808. 9223372036854775807.",
"%d. %d.", &int1, &int2);
test_eq(r,2);
- test_eq(int1, -9223372036854775807-1);
+ test_eq(int1, -9223372036854775807 - 1);
test_eq(int2, 9223372036854775807);
+ /* But INT64_MIN - 1 and INT64_MAX + 1 shouldn't work */
r = tor_sscanf("-9223372036854775809.", "%d.", &int1);
test_eq(r,0);
r = tor_sscanf("9223372036854775808.", "%d.", &int1);
test_eq(r,0);
+
+ /* and the first failure stops further processing */
+ r = tor_sscanf("-9223372036854775808. 9223372036854775808.",
+ "%d. %d.", &int1, &int2);
+ test_eq(r,1);
+
+ r = tor_sscanf("-9223372036854775809. 9223372036854775807.",
+ "%d. %d.", &int1, &int2);
+ test_eq(r,0);
+
+ r = tor_sscanf("9223372036854775808. -9223372036854775809.",
+ "%d. %d.", &int1, &int2);
+ test_eq(r,0);
#endif
#if SIZEOF_LONG == 4
+ /* %lu */
+ /* UINT32_MAX should work */
+ test_eq(1, tor_sscanf("4294967295", "%lu", &ulng));
+ test_eq(4294967295UL, ulng);
+
+ /* But UINT32_MAX + 1 shouldn't work */
+ test_eq(0, tor_sscanf("4294967296", "%lu", &ulng));
+ /* but parsing only 9... */
+ test_eq(1, tor_sscanf("4294967296", "%9lu", &ulng));
+ test_eq(429496729UL, ulng);
+
+ /* %lx */
+ /* UINT32_MAX should work */
+ test_eq(1, tor_sscanf("FFFFFFFF", "%lx", &ulng));
+ test_eq(0xFFFFFFFFUL, ulng);
+
+ /* But UINT32_MAX + 1 shouldn't work */
+ test_eq(0, tor_sscanf("100000000", "%lx", &ulng));
+
+ /* %ld */
+ /* INT32_MIN and INT32_MAX should work */
r = tor_sscanf("-2147483648. 2147483647.", "%ld. %ld.", &lng1, &lng2);
test_eq(r,2);
- test_eq(lng1, -2147483647 - 1);
- test_eq(lng2, 2147483647);
+ test_eq(lng1, -2147483647L - 1L);
+ test_eq(lng2, 2147483647L);
+
+ /* But INT32_MIN - 1 and INT32_MAX + 1 shouldn't work */
+ r = tor_sscanf("-2147483649.", "%ld.", &lng1);
+ test_eq(r,0);
+
+ r = tor_sscanf("2147483648.", "%ld.", &lng1);
+ test_eq(r,0);
+
+ /* and the first failure stops further processing */
+ r = tor_sscanf("-2147483648. 2147483648.",
+ "%ld. %ld.", &lng1, &lng2);
+ test_eq(r,1);
+
+ r = tor_sscanf("-2147483649. 2147483647.",
+ "%ld. %ld.", &lng1, &lng2);
+ test_eq(r,0);
+
+ r = tor_sscanf("2147483648. -2147483649.",
+ "%ld. %ld.", &lng1, &lng2);
+ test_eq(r,0);
#elif SIZEOF_LONG == 8
+ /* %lu */
+ /* UINT64_MAX should work */
+ test_eq(1, tor_sscanf("18446744073709551615", "%lu", &ulng));
+ test_eq(18446744073709551615UL, ulng);
+
+ /* But UINT64_MAX + 1 shouldn't work */
+ test_eq(0, tor_sscanf("18446744073709551616", "%lu", &ulng));
+ /* but parsing only 19... */
+ test_eq(1, tor_sscanf("18446744073709551616", "%19lu", &ulng));
+ test_eq(1844674407370955161UL, ulng);
+
+ /* %lx */
+ /* UINT64_MAX should work */
+ test_eq(1, tor_sscanf("FFFFFFFFFFFFFFFF", "%lx", &ulng));
+ test_eq(0xFFFFFFFFFFFFFFFFUL, ulng);
+
+ /* But UINT64_MAX + 1 shouldn't work */
+ test_eq(0, tor_sscanf("10000000000000000", "%lx", &ulng));
+
+ /* %ld */
+ /* INT64_MIN and INT64_MAX should work */
r = tor_sscanf("-9223372036854775808. 9223372036854775807.",
"%ld. %ld.", &lng1, &lng2);
test_eq(r,2);
- test_eq(lng1, -9223372036854775807L - 1);
+ test_eq(lng1, -9223372036854775807L - 1L);
test_eq(lng2, 9223372036854775807L);
+ /* But INT64_MIN - 1 and INT64_MAX + 1 shouldn't work */
+ r = tor_sscanf("-9223372036854775809.", "%ld.", &lng1);
+ test_eq(r,0);
+
+ r = tor_sscanf("9223372036854775808.", "%ld.", &lng1);
+ test_eq(r,0);
+
+ /* and the first failure stops further processing */
r = tor_sscanf("-9223372036854775808. 9223372036854775808.",
"%ld. %ld.", &lng1, &lng2);
test_eq(r,1);
- r = tor_sscanf("-9223372036854775809. 9223372036854775808.",
+
+ r = tor_sscanf("-9223372036854775809. 9223372036854775807.",
+ "%ld. %ld.", &lng1, &lng2);
+ test_eq(r,0);
+
+ r = tor_sscanf("9223372036854775808. -9223372036854775809.",
"%ld. %ld.", &lng1, &lng2);
test_eq(r,0);
#endif
More information about the tor-commits
mailing list