Commit 48f04a82 authored by Yossi Gottlieb's avatar Yossi Gottlieb Committed by Oran Agra
Browse files

Fix integer overflow (CVE-2021-21309). (#8522)

On 32-bit systems, setting the proto-max-bulk-len config parameter to a high value may result with integer overflow and a subsequent heap overflow when parsing an input bulk (CVE-2021-21309).

This fix has two parts:

Set a reasonable limit to the config parameter.
Add additional checks to prevent the problem in other potential but unknown code paths.

(cherry picked from commit d32f2e99)

Fix MSVR reported issue.
parent e461f599
...@@ -881,10 +881,10 @@ void loadServerConfig(char *filename, char *options) { ...@@ -881,10 +881,10 @@ void loadServerConfig(char *filename, char *options) {
if (max != LLONG_MAX && ll > max) goto badfmt; \ if (max != LLONG_MAX && ll > max) goto badfmt; \
_var = ll; _var = ll;
#define config_set_memory_field(_name,_var) \ #define config_set_memory_field(_name,_var,min,max) \
} else if (!strcasecmp(c->argv[2]->ptr,_name)) { \ } else if (!strcasecmp(c->argv[2]->ptr,_name)) { \
ll = memtoll(o->ptr,&err); \ ll = memtoll(o->ptr,&err); \
if (err || ll < 0) goto badfmt; \ if (err || ll < (long long) (min) || ll > (long long) (max)) goto badfmt; \
_var = ll; _var = ll;
#define config_set_enum_field(_name,_var,_enumvar) \ #define config_set_enum_field(_name,_var,_enumvar) \
...@@ -1150,7 +1150,7 @@ void configSetCommand(client *c) { ...@@ -1150,7 +1150,7 @@ void configSetCommand(client *c) {
} config_set_numerical_field( } config_set_numerical_field(
"active-defrag-threshold-upper",server.active_defrag_threshold_upper,0,1000) { "active-defrag-threshold-upper",server.active_defrag_threshold_upper,0,1000) {
} config_set_memory_field( } config_set_memory_field(
"active-defrag-ignore-bytes",server.active_defrag_ignore_bytes) { "active-defrag-ignore-bytes",server.active_defrag_ignore_bytes,0,LONG_MAX) {
} config_set_numerical_field( } config_set_numerical_field(
"active-defrag-cycle-min",server.active_defrag_cycle_min,1,99) { "active-defrag-cycle-min",server.active_defrag_cycle_min,1,99) {
} config_set_numerical_field( } config_set_numerical_field(
...@@ -1246,7 +1246,7 @@ void configSetCommand(client *c) { ...@@ -1246,7 +1246,7 @@ void configSetCommand(client *c) {
/* Memory fields. /* Memory fields.
* config_set_memory_field(name,var) */ * config_set_memory_field(name,var) */
} config_set_memory_field("maxmemory",server.maxmemory) { } config_set_memory_field("maxmemory",server.maxmemory,0,LONG_MAX) {
if (server.maxmemory) { if (server.maxmemory) {
if (server.maxmemory < zmalloc_used_memory()) { if (server.maxmemory < zmalloc_used_memory()) {
serverLog(LL_WARNING,"WARNING: the new maxmemory value set via CONFIG SET is smaller than the current memory usage. This will result in key eviction and/or the inability to accept new write commands depending on the maxmemory-policy."); serverLog(LL_WARNING,"WARNING: the new maxmemory value set via CONFIG SET is smaller than the current memory usage. This will result in key eviction and/or the inability to accept new write commands depending on the maxmemory-policy.");
...@@ -1254,12 +1254,12 @@ void configSetCommand(client *c) { ...@@ -1254,12 +1254,12 @@ void configSetCommand(client *c) {
freeMemoryIfNeededAndSafe(); freeMemoryIfNeededAndSafe();
} }
} config_set_memory_field( } config_set_memory_field(
"proto-max-bulk-len",server.proto_max_bulk_len) { "proto-max-bulk-len",server.proto_max_bulk_len,1024*1024,LONG_MAX/2) {
} config_set_memory_field( } config_set_memory_field(
"client-query-buffer-limit",server.client_max_querybuf_len) { "client-query-buffer-limit",server.client_max_querybuf_len,0,LONG_MAX) {
} config_set_memory_field("repl-backlog-size",ll) { } config_set_memory_field("repl-backlog-size",ll,0,LONG_MAX) {
resizeReplicationBacklog(ll); resizeReplicationBacklog(ll);
} config_set_memory_field("auto-aof-rewrite-min-size",ll) { } config_set_memory_field("auto-aof-rewrite-min-size",ll,0,LONG_MAX) {
server.aof_rewrite_min_size = ll; server.aof_rewrite_min_size = ll;
/* Enumeration fields. /* Enumeration fields.
......
...@@ -96,6 +96,7 @@ sds sdsnewlen(const void *init, size_t initlen) { ...@@ -96,6 +96,7 @@ sds sdsnewlen(const void *init, size_t initlen) {
int hdrlen = sdsHdrSize(type); int hdrlen = sdsHdrSize(type);
unsigned char *fp; /* flags pointer. */ unsigned char *fp; /* flags pointer. */
assert(hdrlen+initlen+1 > initlen); /* Catch size_t overflow */
sh = s_malloc(hdrlen+initlen+1); sh = s_malloc(hdrlen+initlen+1);
if (init==SDS_NOINIT) if (init==SDS_NOINIT)
init = NULL; init = NULL;
...@@ -214,6 +215,7 @@ sds sdsMakeRoomFor(sds s, size_t addlen) { ...@@ -214,6 +215,7 @@ sds sdsMakeRoomFor(sds s, size_t addlen) {
len = sdslen(s); len = sdslen(s);
sh = (char*)s-sdsHdrSize(oldtype); sh = (char*)s-sdsHdrSize(oldtype);
newlen = (len+addlen); newlen = (len+addlen);
assert(newlen > len); /* Catch size_t overflow */
if (newlen < SDS_MAX_PREALLOC) if (newlen < SDS_MAX_PREALLOC)
newlen *= 2; newlen *= 2;
else else
...@@ -227,6 +229,7 @@ sds sdsMakeRoomFor(sds s, size_t addlen) { ...@@ -227,6 +229,7 @@ sds sdsMakeRoomFor(sds s, size_t addlen) {
if (type == SDS_TYPE_5) type = SDS_TYPE_8; if (type == SDS_TYPE_5) type = SDS_TYPE_8;
hdrlen = sdsHdrSize(type); hdrlen = sdsHdrSize(type);
assert(hdrlen+newlen+1 > len); /* Catch size_t overflow */
if (oldtype==type) { if (oldtype==type) {
newsh = s_realloc(sh, hdrlen+newlen+1); newsh = s_realloc(sh, hdrlen+newlen+1);
if (newsh == NULL) return NULL; if (newsh == NULL) return NULL;
......
...@@ -56,6 +56,12 @@ void zlibc_free(void *ptr) { ...@@ -56,6 +56,12 @@ void zlibc_free(void *ptr) {
#endif #endif
#endif #endif
#if PREFIX_SIZE > 0
#define ASSERT_NO_SIZE_OVERFLOW(sz) assert((sz) + PREFIX_SIZE > (sz))
#else
#define ASSERT_NO_SIZE_OVERFLOW(sz)
#endif
/* Explicitly override malloc/free etc when using tcmalloc. */ /* Explicitly override malloc/free etc when using tcmalloc. */
#if defined(USE_TCMALLOC) #if defined(USE_TCMALLOC)
#define malloc(size) tc_malloc(size) #define malloc(size) tc_malloc(size)
...@@ -96,6 +102,7 @@ static void zmalloc_default_oom(size_t size) { ...@@ -96,6 +102,7 @@ static void zmalloc_default_oom(size_t size) {
static void (*zmalloc_oom_handler)(size_t) = zmalloc_default_oom; static void (*zmalloc_oom_handler)(size_t) = zmalloc_default_oom;
void *zmalloc(size_t size) { void *zmalloc(size_t size) {
ASSERT_NO_SIZE_OVERFLOW(size);
void *ptr = malloc(size+PREFIX_SIZE); void *ptr = malloc(size+PREFIX_SIZE);
if (!ptr) zmalloc_oom_handler(size); if (!ptr) zmalloc_oom_handler(size);
...@@ -114,6 +121,7 @@ void *zmalloc(size_t size) { ...@@ -114,6 +121,7 @@ void *zmalloc(size_t size) {
* Currently implemented only for jemalloc. Used for online defragmentation. */ * Currently implemented only for jemalloc. Used for online defragmentation. */
#ifdef HAVE_DEFRAG #ifdef HAVE_DEFRAG
void *zmalloc_no_tcache(size_t size) { void *zmalloc_no_tcache(size_t size) {
ASSERT_NO_SIZE_OVERFLOW(size);
void *ptr = mallocx(size+PREFIX_SIZE, MALLOCX_TCACHE_NONE); void *ptr = mallocx(size+PREFIX_SIZE, MALLOCX_TCACHE_NONE);
if (!ptr) zmalloc_oom_handler(size); if (!ptr) zmalloc_oom_handler(size);
update_zmalloc_stat_alloc(zmalloc_size(ptr)); update_zmalloc_stat_alloc(zmalloc_size(ptr));
...@@ -128,6 +136,7 @@ void zfree_no_tcache(void *ptr) { ...@@ -128,6 +136,7 @@ void zfree_no_tcache(void *ptr) {
#endif #endif
void *zcalloc(size_t size) { void *zcalloc(size_t size) {
ASSERT_NO_SIZE_OVERFLOW(size);
void *ptr = calloc(1, size+PREFIX_SIZE); void *ptr = calloc(1, size+PREFIX_SIZE);
if (!ptr) zmalloc_oom_handler(size); if (!ptr) zmalloc_oom_handler(size);
...@@ -142,6 +151,7 @@ void *zcalloc(size_t size) { ...@@ -142,6 +151,7 @@ void *zcalloc(size_t size) {
} }
void *zrealloc(void *ptr, size_t size) { void *zrealloc(void *ptr, size_t size) {
ASSERT_NO_SIZE_OVERFLOW(size);
#ifndef HAVE_MALLOC_SIZE #ifndef HAVE_MALLOC_SIZE
void *realptr; void *realptr;
#endif #endif
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment