Lua cjson and cmsgpack integer overflow issues (CVE-2022-24834) (redi…

…s#12398)

* Fix integer overflows due to using wrong integer size.
* Add assertions / panic when overflow still happens.
* Deletion of dead code to avoid need to maintain it
* Some changes are not because of bugs, but rather paranoia.
* Improve cmsgpack and cjson test coverage.

Co-authored-by: Yossi Gottlieb <[email protected]>

deps/Makefile

@@ -3,6 +3,7 @@
 uname_S:= $(shell sh -c 'uname -s 2>/dev/null || echo not')
 
 LUA_DEBUG?=no
+LUA_COVERAGE?=no
 
 CCCOLOR="\033[34m"
 LINKCOLOR="\033[34;1m"
@@ -85,6 +86,11 @@ ifeq ($(LUA_DEBUG),yes)
 else
 	LUA_CFLAGS+= -O2 
 endif
+ifeq ($(LUA_COVERAGE),yes)
+	LUA_CFLAGS += -fprofile-arcs -ftest-coverage
+	LUA_LDFLAGS += -fprofile-arcs -ftest-coverage
+endif
+
 # lua's Makefile defines AR="ar rcu", which is unusual, and makes it more
 # challenging to cross-compile lua (and redis).  These defines make it easier
 # to fit redis into cross-compilation environments, which typically set AR.

deps/lua/src/lua_cjson.c

@@ -39,6 +39,7 @@
 #include <assert.h>
 #include <string.h>
 #include <math.h>
+#include <stdint.h>
 #include <limits.h>
 #include "lua.h"
 #include "lauxlib.h"
@@ -141,13 +142,13 @@ typedef struct {
 
 typedef struct {
     json_token_type_t type;
-    int index;
+    size_t index;
     union {
         const char *string;
         double number;
         int boolean;
     } value;
-    int string_len;
+    size_t string_len;
 } json_token_t;
 
 static const char *char2escape[256] = {
@@ -473,6 +474,8 @@ static void json_append_string(lua_State *l, strbuf_t *json, int lindex)
      * This buffer is reused constantly for small strings
      * If there are any excess pages, they won't be hit anyway.
      * This gains ~5% speedup. */
+    if (len > SIZE_MAX / 6 - 3)
+        abort(); /* Overflow check */
     strbuf_ensure_empty_length(json, len * 6 + 2);
 
     strbuf_append_char_unsafe(json, '\"');
@@ -706,7 +709,7 @@ static int json_encode(lua_State *l)
     strbuf_t local_encode_buf;
     strbuf_t *encode_buf;
     char *json;
-    int len;
+    size_t len;
 
     luaL_argcheck(l, lua_gettop(l) == 1, 1, "expected 1 argument");
 

deps/lua/src/lua_cmsgpack.c

@@ -117,7 +117,9 @@ mp_buf *mp_buf_new(lua_State *L) {
 
 void mp_buf_append(lua_State *L, mp_buf *buf, const unsigned char *s, size_t len) {
     if (buf->free < len) {
-        size_t newsize = (buf->len+len)*2;
+        size_t newsize = buf->len+len;
+        if (newsize < buf->len || newsize >= SIZE_MAX/2) abort();
+        newsize *= 2;
 
         buf->b = (unsigned char*)mp_realloc(L, buf->b, buf->len + buf->free, newsize);
         buf->free = newsize - buf->len;
@@ -173,7 +175,7 @@ void mp_cur_init(mp_cur *cursor, const unsigned char *s, size_t len) {
 
 void mp_encode_bytes(lua_State *L, mp_buf *buf, const unsigned char *s, size_t len) {
     unsigned char hdr[5];
-    int hdrlen;
+    size_t hdrlen;
 
     if (len < 32) {
         hdr[0] = 0xa0 | (len&0xff); /* fix raw */
@@ -220,7 +222,7 @@ void mp_encode_double(lua_State *L, mp_buf *buf, double d) {
 
 void mp_encode_int(lua_State *L, mp_buf *buf, int64_t n) {
     unsigned char b[9];
-    int enclen;
+    size_t enclen;
 
     if (n >= 0) {
         if (n <= 127) {
@@ -290,9 +292,9 @@ void mp_encode_int(lua_State *L, mp_buf *buf, int64_t n) {
     mp_buf_append(L,buf,b,enclen);
 }
 
-void mp_encode_array(lua_State *L, mp_buf *buf, int64_t n) {
+void mp_encode_array(lua_State *L, mp_buf *buf, uint64_t n) {
     unsigned char b[5];
-    int enclen;
+    size_t enclen;
 
     if (n <= 15) {
         b[0] = 0x90 | (n & 0xf);    /* fix array */
@@ -313,7 +315,7 @@ void mp_encode_array(lua_State *L, mp_buf *buf, int64_t n) {
     mp_buf_append(L,buf,b,enclen);
 }
 
-void mp_encode_map(lua_State *L, mp_buf *buf, int64_t n) {
+void mp_encode_map(lua_State *L, mp_buf *buf, uint64_t n) {
     unsigned char b[5];
     int enclen;
 
@@ -791,7 +793,7 @@ void mp_decode_to_lua_type(lua_State *L, mp_cur *c) {
     }
 }
 
-int mp_unpack_full(lua_State *L, int limit, int offset) {
+int mp_unpack_full(lua_State *L, lua_Integer limit, lua_Integer offset) {
     size_t len;
     const char *s;
     mp_cur c;
@@ -803,10 +805,10 @@ int mp_unpack_full(lua_State *L, int limit, int offset) {
     if (offset < 0 || limit < 0) /* requesting negative off or lim is invalid */
         return luaL_error(L,
             "Invalid request to unpack with offset of %d and limit of %d.",
-            offset, len);
+            (int) offset, (int) len);
     else if (offset > len)
         return luaL_error(L,
-            "Start offset %d greater than input length %d.", offset, len);
+            "Start offset %d greater than input length %d.", (int) offset, (int) len);
 
     if (decode_all) limit = INT_MAX;
 
@@ -828,12 +830,13 @@ int mp_unpack_full(lua_State *L, int limit, int offset) {
         /* c->left is the remaining size of the input buffer.
          * subtract the entire buffer size from the unprocessed size
          * to get our next start offset */
-        int offset = len - c.left;
+        size_t new_offset = len - c.left;
+        if (new_offset > LONG_MAX) abort();
 
         luaL_checkstack(L, 1, "in function mp_unpack_full");
 
         /* Return offset -1 when we have have processed the entire buffer. */
-        lua_pushinteger(L, c.left == 0 ? -1 : offset);
+        lua_pushinteger(L, c.left == 0 ? -1 : (lua_Integer) new_offset);
         /* Results are returned with the arg elements still
          * in place. Lua takes care of only returning
          * elements above the args for us.
@@ -852,15 +855,15 @@ int mp_unpack(lua_State *L) {
 }
 
 int mp_unpack_one(lua_State *L) {
-    int offset = luaL_optinteger(L, 2, 0);
+    lua_Integer offset = luaL_optinteger(L, 2, 0);
     /* Variable pop because offset may not exist */
     lua_pop(L, lua_gettop(L)-1);
     return mp_unpack_full(L, 1, offset);
 }
 
 int mp_unpack_limit(lua_State *L) {
-    int limit = luaL_checkinteger(L, 2);
-    int offset = luaL_optinteger(L, 3, 0);
+    lua_Integer limit = luaL_checkinteger(L, 2);
+    lua_Integer offset = luaL_optinteger(L, 3, 0);
     /* Variable pop because offset may not exist */
     lua_pop(L, lua_gettop(L)-1);
 

deps/lua/src/strbuf.c

@@ -26,6 +26,7 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <string.h>
+#include <stdint.h>
 
 #include "strbuf.h"
 
@@ -38,22 +39,22 @@ static void die(const char *fmt, ...)
     va_end(arg);
     fprintf(stderr, "\n");
 
-    exit(-1);
+    abort();
 }
 
-void strbuf_init(strbuf_t *s, int len)
+void strbuf_init(strbuf_t *s, size_t len)
 {
-    int size;
+    size_t size;
 
-    if (len <= 0)
+    if (!len)
         size = STRBUF_DEFAULT_SIZE;
     else
-        size = len + 1;         /* \0 terminator */
-
+        size = len + 1;
+    if (size < len)
+        die("Overflow, len: %zu", len);
     s->buf = NULL;
     s->size = size;
     s->length = 0;
-    s->increment = STRBUF_DEFAULT_INCREMENT;
     s->dynamic = 0;
     s->reallocs = 0;
     s->debug = 0;
@@ -65,7 +66,7 @@ void strbuf_init(strbuf_t *s, int len)
     strbuf_ensure_null(s);
 }
 
-strbuf_t *strbuf_new(int len)
+strbuf_t *strbuf_new(size_t len)
 {
     strbuf_t *s;
 
@@ -81,20 +82,10 @@ strbuf_t *strbuf_new(int len)
     return s;
 }
 
-void strbuf_set_increment(strbuf_t *s, int increment)
-{
-    /* Increment > 0:  Linear buffer growth rate
-     * Increment < -1: Exponential buffer growth rate */
-    if (increment == 0 || increment == -1)
-        die("BUG: Invalid string increment");
-
-    s->increment = increment;
-}
-
 static inline void debug_stats(strbuf_t *s)
 {
     if (s->debug) {
-        fprintf(stderr, "strbuf(%lx) reallocs: %d, length: %d, size: %d\n",
+        fprintf(stderr, "strbuf(%lx) reallocs: %d, length: %zd, size: %zd\n",
                 (long)s, s->reallocs, s->length, s->size);
     }
 }
@@ -113,7 +104,7 @@ void strbuf_free(strbuf_t *s)
         free(s);
 }
 
-char *strbuf_free_to_string(strbuf_t *s, int *len)
+char *strbuf_free_to_string(strbuf_t *s, size_t *len)
 {
     char *buf;
 
@@ -131,57 +122,62 @@ char *strbuf_free_to_string(strbuf_t *s, int *len)
     return buf;
 }
 
-static int calculate_new_size(strbuf_t *s, int len)
+static size_t calculate_new_size(strbuf_t *s, size_t len)
 {
-    int reqsize, newsize;
+    size_t reqsize, newsize;
 
     if (len <= 0)
         die("BUG: Invalid strbuf length requested");
 
     /* Ensure there is room for optional NULL termination */
     reqsize = len + 1;
+    if (reqsize < len)
+        die("Overflow, len: %zu", len);
 
     /* If the user has requested to shrink the buffer, do it exactly */
     if (s->size > reqsize)
         return reqsize;
 
     newsize = s->size;
-    if (s->increment < 0) {
+    if (reqsize >= SIZE_MAX / 2) {
+        newsize = reqsize;
+    } else {
         /* Exponential sizing */
         while (newsize < reqsize)
-            newsize *= -s->increment;
-    } else {
-        /* Linear sizing */
-        newsize = ((newsize + s->increment - 1) / s->increment) * s->increment;
+            newsize *= 2;
     }
 
+    if (newsize < reqsize)
+        die("BUG: strbuf length would overflow, len: %zu", len);
+
     return newsize;
 }
 
 
 /* Ensure strbuf can handle a string length bytes long (ignoring NULL
  * optional termination). */
-void strbuf_resize(strbuf_t *s, int len)
+void strbuf_resize(strbuf_t *s, size_t len)
 {
-    int newsize;
+    size_t newsize;
 
     newsize = calculate_new_size(s, len);
 
     if (s->debug > 1) {
-        fprintf(stderr, "strbuf(%lx) resize: %d => %d\n",
+        fprintf(stderr, "strbuf(%lx) resize: %zd => %zd\n",
                 (long)s, s->size, newsize);
     }
 
     s->size = newsize;
     s->buf = realloc(s->buf, s->size);
     if (!s->buf)
-        die("Out of memory");
+        die("Out of memory, len: %zu", len);
     s->reallocs++;
 }
 
 void strbuf_append_string(strbuf_t *s, const char *str)
 {
-    int space, i;
+    int i;
+    size_t space;
 
     space = strbuf_empty_length(s);
 
@@ -197,55 +193,6 @@ void strbuf_append_string(strbuf_t *s, const char *str)
     }
 }
 
-/* strbuf_append_fmt() should only be used when an upper bound
- * is known for the output string. */
-void strbuf_append_fmt(strbuf_t *s, int len, const char *fmt, ...)
-{
-    va_list arg;
-    int fmt_len;
-
-    strbuf_ensure_empty_length(s, len);
-
-    va_start(arg, fmt);
-    fmt_len = vsnprintf(s->buf + s->length, len, fmt, arg);
-    va_end(arg);
-
-    if (fmt_len < 0)
-        die("BUG: Unable to convert number");  /* This should never happen.. */
-
-    s->length += fmt_len;
-}
-
-/* strbuf_append_fmt_retry() can be used when the there is no known
- * upper bound for the output string. */
-void strbuf_append_fmt_retry(strbuf_t *s, const char *fmt, ...)
-{
-    va_list arg;
-    int fmt_len, try;
-    int empty_len;
-
-    /* If the first attempt to append fails, resize the buffer appropriately
-     * and try again */
-    for (try = 0; ; try++) {
-        va_start(arg, fmt);
-        /* Append the new formatted string */
-        /* fmt_len is the length of the string required, excluding the
-         * trailing NULL */
-        empty_len = strbuf_empty_length(s);
-        /* Add 1 since there is also space to store the terminating NULL. */
-        fmt_len = vsnprintf(s->buf + s->length, empty_len + 1, fmt, arg);
-        va_end(arg);
-
-        if (fmt_len <= empty_len)
-            break;  /* SUCCESS */
-        if (try > 0)
-            die("BUG: length of formatted string changed");
-
-        strbuf_resize(s, s->length + fmt_len);
-    }
-
-    s->length += fmt_len;
-}
 
 /* vi:ai et sw=4 ts=4:
  */