summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--alloc.c4
-rw-r--r--alloc.h5
-rw-r--r--hiredis.c1
-rw-r--r--test.c16
4 files changed, 25 insertions, 1 deletions
diff --git a/alloc.c b/alloc.c
index 7fb6b35..0902286 100644
--- a/alloc.c
+++ b/alloc.c
@@ -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);
}
diff --git a/alloc.h b/alloc.h
index 34a05f4..771f9fe 100644
--- a/alloc.h
+++ b/alloc.h
@@ -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);
}
diff --git a/hiredis.c b/hiredis.c
index 15de4ad..7e7af82 100644
--- a/hiredis.c
+++ b/hiredis.c
@@ -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);
diff --git a/test.c b/test.c
index 9c91107..91feaed 100644
--- a/test.c
+++ b/test.c
@@ -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();
}