From f3a7ad42104e0d25493a06ed3d1d66c7b4fd58bb Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Tue, 18 Jul 2023 16:50:53 +0800 Subject: [PATCH 1/3] bugfix: Lua cjson and cmsgpack integer overflow issues (CVE-2022-24834) * Fix integer overflows due to using wrong integer size. * Add assertions / panic when overflow still happens. Co-authored-by: Yossi Gottlieb --- lua_cjson.c | 9 +++-- strbuf.c | 110 ++++++++++++++-------------------------------------- strbuf.h | 46 +++++++++------------- 3 files changed, 54 insertions(+), 111 deletions(-) diff --git a/lua_cjson.c b/lua_cjson.c index 42672de3..363466c7 100644 --- a/lua_cjson.c +++ b/lua_cjson.c @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -179,13 +180,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] = { @@ -557,6 +558,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, '\"'); @@ -848,7 +851,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"); diff --git a/strbuf.c b/strbuf.c index ed133678..2dc30beb 100644 --- a/strbuf.c +++ b/strbuf.c @@ -26,6 +26,7 @@ #include #include #include +#include #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,63 @@ 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 if (s->increment != 0) { - /* 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 +194,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: */ diff --git a/strbuf.h b/strbuf.h index a98ee220..d77e0f4b 100644 --- a/strbuf.h +++ b/strbuf.h @@ -32,15 +32,13 @@ /* Size: Total bytes allocated to *buf * Length: String length, excluding optional NULL terminator. - * Increment: Allocation increments when resizing the string buffer. * Dynamic: True if created via strbuf_new() */ typedef struct { char *buf; - int size; - int length; - int increment; + size_t size; + size_t length; int dynamic; int reallocs; int debug; @@ -49,33 +47,27 @@ typedef struct { #ifndef STRBUF_DEFAULT_SIZE #define STRBUF_DEFAULT_SIZE 1023 #endif -#ifndef STRBUF_DEFAULT_INCREMENT -#define STRBUF_DEFAULT_INCREMENT -2 -#endif /* Initialise */ -extern strbuf_t *strbuf_new(int len); -extern void strbuf_init(strbuf_t *s, int len); -extern void strbuf_set_increment(strbuf_t *s, int increment); +extern strbuf_t *strbuf_new(size_t len); +extern void strbuf_init(strbuf_t *s, size_t len); /* Release */ extern void strbuf_free(strbuf_t *s); -extern char *strbuf_free_to_string(strbuf_t *s, int *len); +extern char *strbuf_free_to_string(strbuf_t *s, size_t *len); /* Management */ -extern void strbuf_resize(strbuf_t *s, int len); -static int strbuf_empty_length(strbuf_t *s); -static int strbuf_length(strbuf_t *s); -static char *strbuf_string(strbuf_t *s, int *len); -static void strbuf_ensure_empty_length(strbuf_t *s, int len); +extern void strbuf_resize(strbuf_t *s, size_t len); +static size_t strbuf_empty_length(strbuf_t *s); +static size_t strbuf_length(strbuf_t *s); +static char *strbuf_string(strbuf_t *s, size_t *len); +static void strbuf_ensure_empty_length(strbuf_t *s, size_t len); static char *strbuf_empty_ptr(strbuf_t *s); -static void strbuf_extend_length(strbuf_t *s, int len); +static void strbuf_extend_length(strbuf_t *s, size_t len); static void strbuf_set_length(strbuf_t *s, int len); /* Update */ -extern void strbuf_append_fmt(strbuf_t *s, int len, const char *fmt, ...); -extern void strbuf_append_fmt_retry(strbuf_t *s, const char *format, ...); -static void strbuf_append_mem(strbuf_t *s, const char *c, int len); +static void strbuf_append_mem(strbuf_t *s, const char *c, size_t len); extern void strbuf_append_string(strbuf_t *s, const char *str); static void strbuf_append_char(strbuf_t *s, const char c); static void strbuf_ensure_null(strbuf_t *s); @@ -93,12 +85,12 @@ static inline int strbuf_allocated(strbuf_t *s) /* Return bytes remaining in the string buffer * Ensure there is space for a NULL terminator. */ -static inline int strbuf_empty_length(strbuf_t *s) +static inline size_t strbuf_empty_length(strbuf_t *s) { return s->size - s->length - 1; } -static inline void strbuf_ensure_empty_length(strbuf_t *s, int len) +static inline void strbuf_ensure_empty_length(strbuf_t *s, size_t len) { if (len > strbuf_empty_length(s)) strbuf_resize(s, s->length + len); @@ -114,12 +106,12 @@ static inline void strbuf_set_length(strbuf_t *s, int len) s->length = len; } -static inline void strbuf_extend_length(strbuf_t *s, int len) +static inline void strbuf_extend_length(strbuf_t *s, size_t len) { s->length += len; } -static inline int strbuf_length(strbuf_t *s) +static inline size_t strbuf_length(strbuf_t *s) { return s->length; } @@ -135,14 +127,14 @@ static inline void strbuf_append_char_unsafe(strbuf_t *s, const char c) s->buf[s->length++] = c; } -static inline void strbuf_append_mem(strbuf_t *s, const char *c, int len) +static inline void strbuf_append_mem(strbuf_t *s, const char *c, size_t len) { strbuf_ensure_empty_length(s, len); memcpy(s->buf + s->length, c, len); s->length += len; } -static inline void strbuf_append_mem_unsafe(strbuf_t *s, const char *c, int len) +static inline void strbuf_append_mem_unsafe(strbuf_t *s, const char *c, size_t len) { memcpy(s->buf + s->length, c, len); s->length += len; @@ -153,7 +145,7 @@ static inline void strbuf_ensure_null(strbuf_t *s) s->buf[s->length] = 0; } -static inline char *strbuf_string(strbuf_t *s, int *len) +static inline char *strbuf_string(strbuf_t *s, size_t *len) { if (len) *len = s->length; From 55792f9fa83e77f2f91497505fa85feb9971c22d Mon Sep 17 00:00:00 2001 From: lijunlong Date: Thu, 20 Jul 2023 14:55:15 +0800 Subject: [PATCH 2/3] more fixes. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 091ff98b..3867da00 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,7 +27,7 @@ env: - JOBS=3 - LUAROCKS_VER=2.4.2 matrix: - #- LUA=1 LUA_DIR=/usr LUA_INCLUDE_DIR=$LUA_DIR/include/lua5.1 + #- LUA=1 LUA_DIR=/usr LUA_INCLUDE_DIR=$LUA_DIR/include/lua5.1 - LUAJIT=1 LUA_DIR=/usr/local LUA_INCLUDE_DIR=$LUA_DIR/include/luajit-2.1 LUA_SUFFIX=--lua-suffix=jit install: From 364db3d76482302255aeac312fee1c6d04ba1672 Mon Sep 17 00:00:00 2001 From: lijunlong Date: Thu, 20 Jul 2023 23:42:41 +0800 Subject: [PATCH 3/3] revert .travis.yml. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3867da00..469d5dd5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,7 +27,7 @@ env: - JOBS=3 - LUAROCKS_VER=2.4.2 matrix: - #- LUA=1 LUA_DIR=/usr LUA_INCLUDE_DIR=$LUA_DIR/include/lua5.1 + #- LUA=1 LUA_DIR=/usr LUA_INCLUDE_DIR=$LUA_DIR/include/lua5.1 - LUAJIT=1 LUA_DIR=/usr/local LUA_INCLUDE_DIR=$LUA_DIR/include/luajit-2.1 LUA_SUFFIX=--lua-suffix=jit install: