diff options
-rw-r--r-- | alloc.c | 4 | ||||
-rw-r--r-- | alloc.h | 5 | ||||
-rw-r--r-- | hiredis.c | 1 | ||||
-rw-r--r-- | test.c | 16 |
4 files changed, 25 insertions, 1 deletions
@@ -68,6 +68,10 @@ void *hi_malloc(size_t size) { } void *hi_calloc(size_t nmemb, size_t size) { + /* Overflow check as the user can specify any arbitrary allocator */ + if (SIZE_MAX / size < nmemb) + return NULL; + return hiredisAllocFns.callocFn(nmemb, size); } @@ -32,6 +32,7 @@ #define HIREDIS_ALLOC_H #include <stddef.h> /* for size_t */ +#include <stdint.h> #ifdef __cplusplus extern "C" { @@ -59,6 +60,10 @@ static inline void *hi_malloc(size_t size) { } static inline void *hi_calloc(size_t nmemb, size_t size) { + /* Overflow check as the user can specify any arbitrary allocator */ + if (SIZE_MAX / size < nmemb) + return NULL; + return hiredisAllocFns.callocFn(nmemb, size); } @@ -178,7 +178,6 @@ static void *createArrayObject(const redisReadTask *task, size_t elements) { return NULL; if (elements > 0) { - if (SIZE_MAX / sizeof(redisReply*) < elements) return NULL; /* Don't overflow */ r->element = hi_calloc(elements,sizeof(redisReply*)); if (r->element == NULL) { freeReplyObject(r); @@ -59,6 +59,8 @@ struct pushCounters { int str; }; +static int insecure_calloc_calls; + #ifdef HIREDIS_TEST_SSL redisSSLContext *_ssl_ctx = NULL; #endif @@ -765,6 +767,11 @@ static void *hi_calloc_fail(size_t nmemb, size_t size) { return NULL; } +static void *hi_calloc_insecure(size_t nmemb, size_t size) { + insecure_calloc_calls++; + return (void*)0xdeadc0de; +} + static void *hi_realloc_fail(void *ptr, size_t size) { (void)ptr; (void)size; @@ -772,6 +779,8 @@ static void *hi_realloc_fail(void *ptr, size_t size) { } static void test_allocator_injection(void) { + void *ptr; + hiredisAllocFuncs ha = { .mallocFn = hi_malloc_fail, .callocFn = hi_calloc_fail, @@ -791,6 +800,13 @@ static void test_allocator_injection(void) { redisReader *reader = redisReaderCreate(); test_cond(reader == NULL); + /* Make sure hiredis itself protects against a non-overflow checking calloc */ + test("hiredis calloc wrapper protects against overflow: "); + ha.callocFn = hi_calloc_insecure; + hiredisSetAllocators(&ha); + ptr = hi_calloc((SIZE_MAX / sizeof(void*)) + 3, sizeof(void*)); + test_cond(ptr == NULL && insecure_calloc_calls == 0); + // Return allocators to default hiredisResetAllocators(); } |