Skip to content

Commit

Permalink
Bugfix: Lua cjson integer overflow issues (CVE-2022-24834) (#94)
Browse files Browse the repository at this point in the history
* Fix integer overflows due to using wrong integer size.
* Add assertions / panic when overflow still happens.

Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Yossi Gottlieb <yossigo@gmail.com>
  • Loading branch information
3 people authored Jul 20, 2023
1 parent 881accc commit 2bfad8f
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 112 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 6 additions & 3 deletions lua_cjson.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <stdint.h>
#include <string.h>
#include <math.h>
#include <stdint.h>
#include <limits.h>
#include <lua.h>
#include <lauxlib.h>
Expand Down Expand Up @@ -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] = {
Expand Down Expand Up @@ -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, '\"');
Expand Down Expand Up @@ -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");

Expand Down
110 changes: 29 additions & 81 deletions strbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <stdlib.h>
#include <stdarg.h>
#include <string.h>
#include <stdint.h>

#include "strbuf.h"

Expand All @@ -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;
Expand All @@ -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;

Expand All @@ -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);
}
}
Expand All @@ -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;

Expand All @@ -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);

Expand All @@ -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:
*/
46 changes: 19 additions & 27 deletions strbuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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;
}
Expand All @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit 2bfad8f

Please sign in to comment.