antirez / lua-cmsgpack

A self contained Lua MessagePack C implementation.
355 stars 118 forks source link

Lua stack corruption bug #57

Open BigJim opened 7 years ago

BigJim commented 7 years ago

On a "mp_pack()" call with a large depth of nested tables cmsgpack will crash Lua.

The function "mp_encode_lua_table()" grows the Lua stack by two per call. In a recursive situation (reaching the max allowed table depth) such as set in motion by the "test.lua" "Regression test for issue #4, cyclic references in tables.", the Lua stack space can and will usually get exhausted resulting in a crash/segfault/exception from Lua heap corruption.

It's particularly noticeable with a higher value than the default value of 16 for "LUACMSGPACK_MAX_NESTING" is set. Double that to 32 and "test.lua" will crash your Lua. It doesn't crash currently at 16 because the default lua stack is 40 if you have a single argument or two.

#define LUA_MINSTACK 20
#define BASIC_STACK_SIZE  (2*LUA_MINSTACK)

Where to catch it: In "mp_encode_lua_type()" put a breakpoint on the "t = LUA_TNIL;" line. When hit look at the Lua stack size and you will see it equals (LUACMSGPACK_MAX_NESTING x 2) + arg size. Where arg is the current working stack size on entry to "mp_pack()".

A solution that fixes the problem, add to the top of "mp_pack()": luaL_checkstack(L, (LUACMSGPACK_MAX_NESTING * 2), "Can't grow stack.");

This will ensure enough stack space when hitting the nested table limit.

Also I'm sure you could add a similar line (like you have in "mp_decode_to_lua_type()", except growing by two) at the top of "mp_encode_lua_table()" et al.

Update: What you can do (and what I should have had) was to have "LUA_USE_APICHECK" defined for Lua debug builds. Doing so enables an internal stack overflow check in Lua that asserts on hit.