commit a23fd1578612051a3ac804c12a629f6a5cfa296e Author: Hans Jerry Illikainen hji@dyntopia.com Date: Sun Dec 11 20:17:49 2016 +0000

Fix unreachable heap corruption in base64_decode()

Give size_mul_check() external linkage and use it in base64_decode() to avoid a potential integer wrap.

Closes #19222 --- src/common/util.c | 11 +---------- src/common/util.h | 4 +--- src/common/util_format.c | 2 +- src/test/test_util.c | 30 +++++++++++++++--------------- src/test/test_util_format.c | 3 +++ 5 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c index 9d134c1..d02eb66 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -188,7 +188,7 @@ tor_malloc_zero_(size_t size DMALLOC_PARAMS) #define SQRT_SIZE_MAX_P1 (((size_t)1) << (sizeof(size_t)*4))

/** Return non-zero if and only if the product of the arguments is exact. */ -static inline int +inline int size_mul_check(const size_t x, const size_t y) { /* This first check is equivalent to @@ -202,15 +202,6 @@ size_mul_check(const size_t x, const size_t y) x <= SIZE_MAX / y); }

-#ifdef TOR_UNIT_TESTS -/** Exposed for unit tests only */ -int -size_mul_check__(const size_t x, const size_t y) -{ - return size_mul_check(x,y); -} -#endif - /** Allocate a chunk of <b>nmemb</b>*<b>size</b> bytes of memory, fill * the memory with zero bytes, and return a pointer to the result. * Log and terminate the process on error. (Same as diff --git a/src/common/util.h b/src/common/util.h index 2b3e48e..ff56615 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -553,9 +553,7 @@ STATIC int format_helper_exit_status(unsigned char child_state,

#endif

-#ifdef TOR_UNIT_TESTS -int size_mul_check__(const size_t x, const size_t y); -#endif +int size_mul_check(const size_t x, const size_t y);

#define ARRAY_LENGTH(x) ((sizeof(x)) / sizeof(x[0]))

diff --git a/src/common/util_format.c b/src/common/util_format.c index aef9db8..6e0a045 100644 --- a/src/common/util_format.c +++ b/src/common/util_format.c @@ -398,7 +398,7 @@ base64_decode(char *dest, size_t destlen, const char *src, size_t srclen) * Number of bytes required to hold all bits == (srclen*6)/8. * Yes, we want to round down: anything that hangs over the end of a * byte is padding. */ - if (destlen < (srclen*3)/4) + if (!size_mul_check(srclen, 3) || destlen < (srclen*3)/4) return -1; if (destlen > SIZE_T_CEILING) return -1; diff --git a/src/test/test_util.c b/src/test/test_util.c index 86e3fea..fafb84f 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -5479,26 +5479,26 @@ test_util_calloc_check(void *arg) { (void) arg; /* Easy cases that are good. */ - tt_assert(size_mul_check__(0,0)); - tt_assert(size_mul_check__(0,100)); - tt_assert(size_mul_check__(100,0)); - tt_assert(size_mul_check__(100,100)); + tt_assert(size_mul_check(0,0)); + tt_assert(size_mul_check(0,100)); + tt_assert(size_mul_check(100,0)); + tt_assert(size_mul_check(100,100));

/* Harder cases that are still good. */ - tt_assert(size_mul_check__(SIZE_MAX, 1)); - tt_assert(size_mul_check__(1, SIZE_MAX)); - tt_assert(size_mul_check__(SIZE_MAX / 10, 9)); - tt_assert(size_mul_check__(11, SIZE_MAX / 12)); + tt_assert(size_mul_check(SIZE_MAX, 1)); + tt_assert(size_mul_check(1, SIZE_MAX)); + tt_assert(size_mul_check(SIZE_MAX / 10, 9)); + tt_assert(size_mul_check(11, SIZE_MAX / 12)); const size_t sqrt_size_max_p1 = ((size_t)1) << (sizeof(size_t) * 4); - tt_assert(size_mul_check__(sqrt_size_max_p1, sqrt_size_max_p1 - 1)); + tt_assert(size_mul_check(sqrt_size_max_p1, sqrt_size_max_p1 - 1));

/* Cases that overflow */ - tt_assert(! size_mul_check__(SIZE_MAX, 2)); - tt_assert(! size_mul_check__(2, SIZE_MAX)); - tt_assert(! size_mul_check__(SIZE_MAX / 10, 11)); - tt_assert(! size_mul_check__(11, SIZE_MAX / 10)); - tt_assert(! size_mul_check__(SIZE_MAX / 8, 9)); - tt_assert(! size_mul_check__(sqrt_size_max_p1, sqrt_size_max_p1)); + tt_assert(! size_mul_check(SIZE_MAX, 2)); + tt_assert(! size_mul_check(2, SIZE_MAX)); + tt_assert(! size_mul_check(SIZE_MAX / 10, 11)); + tt_assert(! size_mul_check(11, SIZE_MAX / 10)); + tt_assert(! size_mul_check(SIZE_MAX / 8, 9)); + tt_assert(! size_mul_check(sqrt_size_max_p1, sqrt_size_max_p1));

done: ; diff --git a/src/test/test_util_format.c b/src/test/test_util_format.c index 1d58ba2..21a6923 100644 --- a/src/test/test_util_format.c +++ b/src/test/test_util_format.c @@ -202,6 +202,9 @@ test_util_format_base64_decode(void *ignored) res = base64_decode(dst, SIZE_T_CEILING+1, src, 10); tt_int_op(res, OP_EQ, -1);

+ res = base64_decode(dst, 1, real_src, SIZE_MAX/3+1); + tt_int_op(res, OP_EQ, -1); + const char *s = "T3BhIG11bmRv"; res = base64_decode(dst, 9, s, strlen(s)); tt_int_op(res, OP_EQ, 9);