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
This commit is contained in:
Hans Jerry Illikainen 2016-12-11 20:17:49 +00:00 committed by Nick Mathewson
parent f3da62dbdf
commit a23fd15786
5 changed files with 21 additions and 29 deletions

View File

@ -188,7 +188,7 @@ tor_malloc_zero_(size_t size DMALLOC_PARAMS)
#define SQRT_SIZE_MAX_P1 (((size_t)1) << (sizeof(size_t)*4)) #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. */ /** 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) size_mul_check(const size_t x, const size_t y)
{ {
/* This first check is equivalent to /* 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); 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 /** 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. * the memory with zero bytes, and return a pointer to the result.
* Log and terminate the process on error. (Same as * Log and terminate the process on error. (Same as

View File

@ -553,9 +553,7 @@ STATIC int format_helper_exit_status(unsigned char child_state,
#endif #endif
#ifdef TOR_UNIT_TESTS int size_mul_check(const size_t x, const size_t y);
int size_mul_check__(const size_t x, const size_t y);
#endif
#define ARRAY_LENGTH(x) ((sizeof(x)) / sizeof(x[0])) #define ARRAY_LENGTH(x) ((sizeof(x)) / sizeof(x[0]))

View File

@ -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. * 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 * Yes, we want to round down: anything that hangs over the end of a
* byte is padding. */ * byte is padding. */
if (destlen < (srclen*3)/4) if (!size_mul_check(srclen, 3) || destlen < (srclen*3)/4)
return -1; return -1;
if (destlen > SIZE_T_CEILING) if (destlen > SIZE_T_CEILING)
return -1; return -1;

View File

@ -5479,26 +5479,26 @@ test_util_calloc_check(void *arg)
{ {
(void) arg; (void) arg;
/* Easy cases that are good. */ /* Easy cases that are good. */
tt_assert(size_mul_check__(0,0)); tt_assert(size_mul_check(0,0));
tt_assert(size_mul_check__(0,100)); tt_assert(size_mul_check(0,100));
tt_assert(size_mul_check__(100,0)); tt_assert(size_mul_check(100,0));
tt_assert(size_mul_check__(100,100)); tt_assert(size_mul_check(100,100));
/* Harder cases that are still good. */ /* Harder cases that are still good. */
tt_assert(size_mul_check__(SIZE_MAX, 1)); tt_assert(size_mul_check(SIZE_MAX, 1));
tt_assert(size_mul_check__(1, SIZE_MAX)); tt_assert(size_mul_check(1, SIZE_MAX));
tt_assert(size_mul_check__(SIZE_MAX / 10, 9)); tt_assert(size_mul_check(SIZE_MAX / 10, 9));
tt_assert(size_mul_check__(11, SIZE_MAX / 12)); tt_assert(size_mul_check(11, SIZE_MAX / 12));
const size_t sqrt_size_max_p1 = ((size_t)1) << (sizeof(size_t) * 4); 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 */ /* Cases that overflow */
tt_assert(! size_mul_check__(SIZE_MAX, 2)); tt_assert(! size_mul_check(SIZE_MAX, 2));
tt_assert(! size_mul_check__(2, SIZE_MAX)); tt_assert(! size_mul_check(2, SIZE_MAX));
tt_assert(! size_mul_check__(SIZE_MAX / 10, 11)); tt_assert(! size_mul_check(SIZE_MAX / 10, 11));
tt_assert(! size_mul_check__(11, SIZE_MAX / 10)); tt_assert(! size_mul_check(11, SIZE_MAX / 10));
tt_assert(! size_mul_check__(SIZE_MAX / 8, 9)); 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(sqrt_size_max_p1, sqrt_size_max_p1));
done: done:
; ;

View File

@ -202,6 +202,9 @@ test_util_format_base64_decode(void *ignored)
res = base64_decode(dst, SIZE_T_CEILING+1, src, 10); res = base64_decode(dst, SIZE_T_CEILING+1, src, 10);
tt_int_op(res, OP_EQ, -1); 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"; const char *s = "T3BhIG11bmRv";
res = base64_decode(dst, 9, s, strlen(s)); res = base64_decode(dst, 9, s, strlen(s));
tt_int_op(res, OP_EQ, 9); tt_int_op(res, OP_EQ, 9);