From bca7083e8285e8e6a4377076a7e432417eafc6d2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 27 Jan 2016 12:26:02 -0500 Subject: avoid integer overflow in and around smartlist_ensure_capacity. This closes bug 18162; bugfix on a45b1315909c9, which fixed a related issue long ago. In addition to the #18162 issues, this fixes a signed integer overflow in smarltist_add_all(), which is probably not so great either. --- src/common/container.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) (limited to 'src/common/container.c') diff --git a/src/common/container.c b/src/common/container.c index eec497a3e6..46d9c2ee79 100644 --- a/src/common/container.c +++ b/src/common/container.c @@ -60,15 +60,17 @@ smartlist_clear(smartlist_t *sl) /** Make sure that sl can hold at least size entries. */ static INLINE void -smartlist_ensure_capacity(smartlist_t *sl, int size) +smartlist_ensure_capacity(smartlist_t *sl, size_t size) { #if SIZEOF_SIZE_T > SIZEOF_INT #define MAX_CAPACITY (INT_MAX) #else #define MAX_CAPACITY (int)((SIZE_MAX / (sizeof(void*)))) #endif - if (size > sl->capacity) { - int higher = sl->capacity; + tor_assert(size <= MAX_CAPACITY); + + if (size > (size_t) sl->capacity) { + size_t higher = (size_t) sl->capacity; if (PREDICT_UNLIKELY(size > MAX_CAPACITY/2)) { tor_assert(size <= MAX_CAPACITY); higher = MAX_CAPACITY; @@ -76,7 +78,8 @@ smartlist_ensure_capacity(smartlist_t *sl, int size) while (size > higher) higher *= 2; } - sl->capacity = higher; + tor_assert(higher <= INT_MAX); /* Redundant */ + sl->capacity = (int) higher; sl->list = tor_realloc(sl->list, sizeof(void*)*((size_t)sl->capacity)); } } @@ -85,7 +88,7 @@ smartlist_ensure_capacity(smartlist_t *sl, int size) void smartlist_add(smartlist_t *sl, void *element) { - smartlist_ensure_capacity(sl, sl->num_used+1); + smartlist_ensure_capacity(sl, ((size_t) sl->num_used)+1); sl->list[sl->num_used++] = element; } @@ -93,11 +96,12 @@ smartlist_add(smartlist_t *sl, void *element) void smartlist_add_all(smartlist_t *s1, const smartlist_t *s2) { - int new_size = s1->num_used + s2->num_used; - tor_assert(new_size >= s1->num_used); /* check for overflow. */ + size_t new_size = (size_t)s1->num_used + (size_t)s2->num_used; + tor_assert(new_size >= (size_t) s1->num_used); /* check for overflow. */ smartlist_ensure_capacity(s1, new_size); memcpy(s1->list + s1->num_used, s2->list, s2->num_used*sizeof(void*)); - s1->num_used = new_size; + tor_assert(new_size <= INT_MAX); /* redundant. */ + s1->num_used = (int) new_size; } /** Remove all elements E from sl such that E==element. Preserve @@ -334,7 +338,7 @@ smartlist_insert(smartlist_t *sl, int idx, void *val) if (idx == sl->num_used) { smartlist_add(sl, val); } else { - smartlist_ensure_capacity(sl, sl->num_used+1); + smartlist_ensure_capacity(sl, ((size_t) sl->num_used)+1); /* Move other elements away */ if (idx < sl->num_used) memmove(sl->list + idx + 1, sl->list + idx, -- cgit v1.2.3-54-g00ecf From c2fd64846978290b0e7c7165d7658a5e704eee8f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Feb 2016 12:54:52 -0500 Subject: Make ensure_capacity a bit more pedantically correct Issues noted by cypherpunks on #18162 --- src/common/container.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/common/container.c') diff --git a/src/common/container.c b/src/common/container.c index 46d9c2ee79..b1431dfa90 100644 --- a/src/common/container.c +++ b/src/common/container.c @@ -58,11 +58,16 @@ smartlist_clear(smartlist_t *sl) sl->num_used = 0; } +#if SIZE_MAX < INT_MAX +#error "We don't support systems where size_t is smaller than int." +#endif + /** Make sure that sl can hold at least size entries. */ static INLINE void smartlist_ensure_capacity(smartlist_t *sl, size_t size) { -#if SIZEOF_SIZE_T > SIZEOF_INT + /* Set MAX_CAPACITY to MIN(INT_MAX, SIZE_MAX / sizeof(void*)) */ +#if (SIZE_MAX/SIZEOF_VOID_P) > INT_MAX #define MAX_CAPACITY (INT_MAX) #else #define MAX_CAPACITY (int)((SIZE_MAX / (sizeof(void*)))) -- cgit v1.2.3-54-g00ecf