commit 9b344628ed8f15543dc7780cc2a5cdd1b8f656cf Author: Nick Mathewson nickm@torproject.org Date: Mon May 7 12:25:59 2012 -0400

Handle out-of-range values in tor_parse_* integer functions

The underlying strtoX functions handle overflow by saturating and setting errno to ERANGE. If the min/max arguments to the tor_parse_* functions are equal to the minimum/maximum of the underlying type, then with the old approach, we wouldn't treat a too-large value as genuinely broken.

Found this while looking at bug 5786; bugfix on 19da1f36 (in Tor 0.0.9), which introduced these functions. --- changes/bug5786_range | 8 ++++++++ src/common/util.c | 7 +++++++ src/test/test_util.c | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/changes/bug5786_range b/changes/bug5786_range new file mode 100644 index 0000000..40ac4d2 --- /dev/null +++ b/changes/bug5786_range @@ -0,0 +1,8 @@ + o Minor bugfixes: + - Make our number-parsing functions always treat too-large values + as an error, even when those values exceed the width of the + underlying type. Previously, if the caller provided these + functions with minima or maxima set to the extreme values of the + underlying integer type, these functions would return those + values on overflow rather than treating overflow as an error. + Fix for part of bug 5786; bugfix on Tor 0.0.9. \ No newline at end of file diff --git a/src/common/util.c b/src/common/util.c index e3cd154..7d2fc4d 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -764,6 +764,9 @@ tor_digest256_is_zero(const char *digest) /* Helper: common code to check whether the result of a strtol or strtoul or * strtoll is correct. */ #define CHECK_STRTOX_RESULT() \ + /* Did an overflow occur? */ \ + if (errno == ERANGE) \ + goto err; \ /* Was at least one character converted? */ \ if (endptr == s) \ goto err; \ @@ -800,6 +803,7 @@ tor_parse_long(const char *s, int base, long min, long max, char *endptr; long r;

+ errno = 0; r = strtol(s, &endptr, base); CHECK_STRTOX_RESULT(); } @@ -812,6 +816,7 @@ tor_parse_ulong(const char *s, int base, unsigned long min, char *endptr; unsigned long r;

+ errno = 0; r = strtoul(s, &endptr, base); CHECK_STRTOX_RESULT(); } @@ -823,6 +828,7 @@ tor_parse_double(const char *s, double min, double max, int *ok, char **next) char *endptr; double r;

+ errno = 0; r = strtod(s, &endptr); CHECK_STRTOX_RESULT(); } @@ -836,6 +842,7 @@ tor_parse_uint64(const char *s, int base, uint64_t min, char *endptr; uint64_t r;

+ errno = 0; #ifdef HAVE_STRTOULL r = (uint64_t)strtoull(s, &endptr, base); #elif defined(MS_WINDOWS) diff --git a/src/test/test_util.c b/src/test/test_util.c index 23cd059..ee745c5 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -283,6 +283,21 @@ test_util_strmisc(void) test_assert(i == 1); }

+ { + /* Test tor_parse_* where we overflow/underflow the underlying type. */ + /* This string should overflow 64-bit ints. */ +#define TOOBIG "100000000000000000000000000" + test_eq(0L, tor_parse_long(TOOBIG, 10, LONG_MIN, LONG_MAX, &i, NULL)); + test_eq(i, 0); + test_eq(0L, tor_parse_long("-"TOOBIG, 10, LONG_MIN, LONG_MAX, &i, NULL)); + test_eq(i, 0); + test_eq(0UL, tor_parse_ulong(TOOBIG, 10, 0, ULONG_MAX, &i, NULL)); + test_eq(i, 0); + test_eq(U64_LITERAL(0), tor_parse_uint64(TOOBIG, 10, + 0, UINT64_MAX, &i, NULL)); + test_eq(i, 0); + } + /* Test failing snprintf cases */ test_eq(-1, tor_snprintf(buf, 0, "Foo")); test_eq(-1, tor_snprintf(buf, 2, "Foo"));