Stack overflow in luaO_pushvfstring

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

Stack overflow in luaO_pushvfstring

Yongheng Chen

Hi,

 

We found a stack overflow in lua. Here’s the details:

 

Version:

Lua 5.4.0, git hash c33b1728aeb7dfeec4013562660e07d32697aa6b

 

POC:

function errfunc() xpcall(function() print(xpcall(test, errfunc)) end, errfunc)

    end(function() print(xpcall(test, errfunc)) end)()

 

How to reproduce:

./lua poc.lua

 

Stack dump:

AddressSanitizer:DEADLYSIGNAL

=================================================================

==16336==ERROR: AddressSanitizer: stack-overflow on address 0x7fffad2a9d78 (pc 0x7f9977ca3796 bp 0x7fffad2aa610 sp 0x7fffad2a9d80 T0)

    #0 0x7f9977ca3795  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x73795)

    #1 0x422a0c in luaO_pushvfstring (/home/yongheng/lua_asan/lua+0x422a0c)

    #2 0x411455 in luaG_runerror (/home/yongheng/lua_asan/lua+0x411455)

    #3 0x411595 in luaG_typeerror (/home/yongheng/lua_asan/lua+0x411595)

    #4 0x4138bc in luaD_tryfuncTM (/home/yongheng/lua_asan/lua+0x4138bc)

    #5 0x41480d in luaD_call (/home/yongheng/lua_asan/lua+0x41480d)

    #6 0x415194 in luaD_callnoyield (/home/yongheng/lua_asan/lua+0x415194)

    #7 0x4127d0 in luaD_rawrunprotected (/home/yongheng/lua_asan/lua+0x4127d0)

    #8 0x415d70 in luaD_pcall (/home/yongheng/lua_asan/lua+0x415d70)

    #9 0x40bd47 in lua_pcallk (/home/yongheng/lua_asan/lua+0x40bd47)

    #10 0x45672e in luaB_xpcall (/home/yongheng/lua_asan/lua+0x45672e)

#11 0x414de1 in luaD_call (/home/yongheng/lua_asan/lua+0x414de1)

….

 

Found by: Yongheng Chen and Rui Zhong

 

Best,

Yongheng

 

Sent from Mail for Windows 10

 

Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Luiz Henrique de Figueiredo
> We found a stack overflow in lua. Here’s the details:

On which platform? How was Lua built?
Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Roberto Ierusalimschy
In reply to this post by Yongheng Chen
>    We found a stack overflow in lua. Here’s the details:
>
>     [...]

That may be a problem due to how you compiled Lua. Have you tried a smaller
C-stack limit? Add the following in the beginning of your script and see
how it goes:

    debug.setcstacklimit(1000)

(It may be necessary a limit even smaller than 1000...)

-- Roberto
Reply | Threaded
Open this post in threaded view
|

RE: Stack overflow in luaO_pushvfstring

Yongheng Chen

Hi Reberto,

 

This is tested on Ubuntu16.04. I built it with –fsanitize=address.

 

I didn’t see any instruction on how to add `debug.setcstacklimit`. It seems that it is already set in luaconf.h.

 

Thanks,

 

Yongheng

 

Sent from Mail for Windows 10

 

From: [hidden email]
Sent: Monday, July 6, 2020 8:38 AM
To: [hidden email]
Subject: Re: Stack overflow in luaO_pushvfstring

 

>    We found a stack overflow in lua. Here’s the details:

>

>     [...]

 

That may be a problem due to how you compiled Lua. Have you tried a smaller

C-stack limit? Add the following in the beginning of your script and see

how it goes:

 

    debug.setcstacklimit(1000)

 

(It may be necessary a limit even smaller than 1000...)

 

-- Roberto

 

Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Roberto Ierusalimschy
>    This is tested on Ubuntu16.04. I built it with –fsanitize=address.
>
>    I didn’t see any instruction on how to add `debug.setcstacklimit`. It
>    seems that it is already set in luaconf.h.

I am pretty sure you also didn't see any instruction about adding the
option -fsanitize=address to compile Lua. With this option, the code
consumes more stack than usual, and therefore the stack limit must be
adapted accordingly.

If you prefer, you can do that in luaconf.h, too. I suggested
debug.setcstacklimit as a quick fix without the need to recompile Lua.

-- Roberto
Reply | Threaded
Open this post in threaded view
|

RE: Stack overflow in luaO_pushvfstring

Yongheng Chen

I think I get your point. The POC was reduced by creduce, and the original  poc can actually crash the default build lua on Linux. (not tested on windows and mac)

 

 

The original POC that can crash a default build lua ( just run make on Linux) is:

function errfunc() end function test()

    xpcall(test, coroutine.wrap(function() print(xpcall(test, errfunc)) end))

        end coroutine.wrap(function() print(xpcall(test, errfunc)) end)()

 

With the following poc, lua crashed with an invalid stack address.

 

> I am pretty sure you also didn't see any instruction about adding the

option -fsanitize=address to compile Lua.

 

Yeah, I modify the makefile to add this flag.

 

>. I suggested

debug.setcstacklimit as a quick fix without the need to recompile Lua.

 

Could you give some more detailed instruction about it? I couldn’t find how to do it in the readme in the official website of lua.

 

Best,

Yongheng

 

From: [hidden email]
Sent: Monday, July 6, 2020 10:51 AM
To: [hidden email]
Subject: Re: Stack overflow in luaO_pushvfstring

 

>    This is tested on Ubuntu16.04. I built it with –fsanitize=address.

>

>    I didn’t see any instruction on how to add `debug.setcstacklimit`. It

>    seems that it is already set in luaconf.h.

 

I am pretty sure you also didn't see any instruction about adding the

option -fsanitize=address to compile Lua. With this option, the code

consumes more stack than usual, and therefore the stack limit must be

adapted accordingly.

 

If you prefer, you can do that in luaconf.h, too. I suggested

debug.setcstacklimit as a quick fix without the need to recompile Lua.

 

-- Roberto

 

Reply | Threaded
Open this post in threaded view
|

RE: Stack overflow in luaO_pushvfstring

Yongheng Chen
In reply to this post by Roberto Ierusalimschy

Oh, I just need to run it in the lua shell.

 

I just tested it. After I set the stack limit, lua won’t crash.

 

Best,
Yongheng

 

Sent from Mail for Windows 10

 

From: [hidden email]
Sent: Monday, July 6, 2020 10:51 AM
To: [hidden email]
Subject: Re: Stack overflow in luaO_pushvfstring

 

>    This is tested on Ubuntu16.04. I built it with –fsanitize=address.

>

>    I didn’t see any instruction on how to add `debug.setcstacklimit`. It

>    seems that it is already set in luaconf.h.

 

I am pretty sure you also didn't see any instruction about adding the

option -fsanitize=address to compile Lua. With this option, the code

consumes more stack than usual, and therefore the stack limit must be

adapted accordingly.

 

If you prefer, you can do that in luaconf.h, too. I suggested

debug.setcstacklimit as a quick fix without the need to recompile Lua.

 

-- Roberto

 

Reply | Threaded
Open this post in threaded view
|

RE: Stack overflow in luaO_pushvfstring

Yongheng Chen

But since the original I sent affects the default-build lua, I think this is a problem should be fixed?

 

Best,

Yongheng

 

 

Sent from Mail for Windows 10

 

From: [hidden email]
Sent: Monday, July 6, 2020 11:09 AM
To: [hidden email]
Subject: RE: Stack overflow in luaO_pushvfstring

 

Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Roberto Ierusalimschy
>    But since the original I sent affects the default-build lua, I think this
>    is a problem should be fixed?

Sure. Thanks for the help,

-- Roberto
Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Antonio de Curtis
In reply to this post by Yongheng Chen
seen the problem I tested my release with TDM GCC 9.2 on Win 8.0 32
bit with no options at all except warnings, it crashes

CC= gcc -std=gnu99
# CFLAGS= -march=native -msse2 -Ofast -m32 -Wall -Wextra
-DLUA_COMPAT_5_3 $(SYSCFLAGS) $(MYCFLAGS)
CFLAGS= -Wall -Wextra

Modified to

CC= gcc -std=gnu99 -fsanitize=address

I get this error

C:/Dev-Cpp/TDM-GCC-32/bin/../lib/gcc/mingw32/9.2.0/../../../../mingw32/bin/ld.exe:
cannot find -lasan

(might happen only with this 32 bit release of TDM) but eventually
your fix is not portable.

--
Antonio de Curtis
Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Roberto Ierusalimschy
In reply to this post by Yongheng Chen
>    >. I suggested
>    > debug.setcstacklimit as a quick fix without the need to recompile Lua.
>
>    Could you give some more detailed instruction about it? I couldn’t find
>    how to do it in the readme in the official website of lua.

I think the manual has enough information for this function:

    https://www.lua.org/manual/5.4/manual.html#pdf-debug.setcstacklimit

Have you read it?

-- Roberto
Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Gé Weijers
In reply to this post by Yongheng Chen
On Mon, Jul 6, 2020 at 8:16 AM Yongheng Chen <[hidden email]> wrote:

>
> But since the original I sent affects the default-build lua, I think this is a problem should be fixed?
>
>
>
> Best,
>
> Yongheng
>
>
>
>
>
> Sent from Mail for Windows 10
>
>
>
> From: Yongheng Chen
> Sent: Monday, July 6, 2020 11:09 AM
> To: Lua mailing list
> Subject: RE: Stack overflow in luaO_pushvfstring
>
>

The maximum setting of Lua's stack limit depends on the runtime stack
limit set by the OS. You can usually grow this by using the shell
command "ulimit -s <size-in-kbytes>".

The flag "-fsanitize=address" must generate code that uses a lot of
extra stack space. On my Debian 10 system I can run a program with the
C stack limit at 20000 without crashing. Compiled with
-fsanitize=address the crash happens below 4000. I did not change the
process stack limit, it's set to 8192 Kbytes.



--

Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Andrew Gierth
Here is a module that might be of some use when testing these things.
Compile as usual for a loadable C module and:

  lua -l stackchk -e 'print(stackchk())'

should print the size of the stack frame for lua->lua calls in bytes. On
versions other than 5.4 this should return 0, since previously the C
stack wasn't used for such calls; on amd64 with clang -O2 I get a value
of 224.

I'm curious to know what kind of values people get on other
compilers/platforms.

/* stackchk.c */

#include <stddef.h>
#include <string.h>
#include <math.h>

#include <lua.h>
#include <lauxlib.h>

/* note, must avoid tail calls in r() */
static const char *stack_code =
    "local _ENV, f = nil, ...\n"
    "local function r(d)\n"
    "  if d >= 0 then return r(d-1),d else return f(),0 end\n"
    "end\n"
    "return r\n";

static int getdepth(lua_State *L)
{
    volatile int t = 0;
    lua_pushlightuserdata(L, (void*) &t);
    return 1;
}

static int stackchk(lua_State *L)
{
    int i, n = luaL_optinteger(L, 1, 50);
    void *p[2];
    for (i = 0; i < 2; ++i) {
        lua_pushvalue(L, lua_upvalueindex(1));
        lua_pushinteger(L, (i*n));
        lua_call(L, 1, 1);
        p[i] = lua_touserdata(L, -1);
    }
    lua_pushnumber(L, fabs((double)(p[0] - p[1])) / n);
    return 1;
}

int luaopen_stackchk(lua_State *L)
{
    if (luaL_loadbufferx(L, stack_code, strlen(stack_code), "stackchk", "t"))
        lua_error(L);
    lua_pushcfunction(L, getdepth);
    lua_call(L, 1, 1);
    lua_pushcclosure(L, stackchk, 1);
    return 1;
}

/* end */

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Stefan-2
In reply to this post by Yongheng Chen
> The original POC that can crash a default build lua ( just run make on
> Linux) is:
>
> function errfunc() end function test()
>
>     xpcall(test, coroutine.wrap(function() print(xpcall(test, errfunc))
> end))
>
>         end coroutine.wrap(function() print(xpcall(test, errfunc)) end)()
>
>  
>
> With the following poc, lua crashed with an invalid stack address.
>

Can confirm - this bit crashes the standard build with

gcc -std=gnu99 -O2 -Wall -Wextra -DLUA_COMPAT_5_3 -DLUA_USE_LINUX

on my Linux with ulimit -s below 11264.

Linux db 5.7.0-1-amd64 #1 SMP Debian 5.7.6-1 (2020-06-24) x86_64
gcc (Debian 9.3.0-14) 9.3.0

(same for armhf-cross/quemu)

On Windows 64-bit values below 21MB crash:

cl /nologo /EHsc /O2 lua.c lapi.c lctype.c lfunc.c lmathlib.c loslib.c
ltable.c lundump.c lauxlib.c ldblib.c lgc.c lmem.c lparser.c ltablib.c
lutf8lib.c lbaselib.c ldebug.c linit.c loadlib.c lstate.c ltm.c lvm.c
lcode.c ldo.c liolib.c lobject.c lstring.c lzio.c lcorolib.c ldump.c
llex.c lopcodes.c lstrlib.c /link /STACK:22020096 (21MB)

debug.setcstacklimit(1970) prevents a crash on /STACK:20971520 (20MB)

debug.setcstacklimit(1395) prevents a crash on /STACK:10485760 (10MB)
Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Stefan-2
In reply to this post by Andrew Gierth


Am 07.07.2020 um 02:43 schrieb Andrew Gierth:

> Here is a module that might be of some use when testing these things.
> Compile as usual for a loadable C module and:
>
>   lua -l stackchk -e 'print(stackchk())'
>
> should print the size of the stack frame for lua->lua calls in bytes. On
> versions other than 5.4 this should return 0, since previously the C
> stack wasn't used for such calls; on amd64 with clang -O2 I get a value
> of 224.
>
> I'm curious to know what kind of values people get on other
> compilers/platforms.

Linux db 5.7.0-1-amd64 #1 SMP Debian 5.7.6-1 (2020-06-24) x86_64
gcc (Debian 9.3.0-14) 9.3.0
 => 192

Windows 7 64-bit / MSVC 19.26.28806 for x64 (I hope the flags are right)
 => 480
Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Andrew Gierth
In reply to this post by Yongheng Chen
>>>>> "Yongheng" == Yongheng Chen <[hidden email]> writes:

 Yongheng> The original POC that can crash a default build lua ( just
 Yongheng> run make on Linux) is:

 Yongheng> function errfunc() end function test()
 Yongheng>     xpcall(test, coroutine.wrap(function() print(xpcall(test, errfunc)) end))
 Yongheng>         end coroutine.wrap(function() print(xpcall(test, errfunc)) end)()

So surely this, in lua_resume, can't possibly be right:

        L->nCcalls = getCcalls(from) + from->nci - L->nci - CSTACKCF;

We get here when the stack overflow error trips the first time and we
try and resume the wrapped coroutine which is supplied as the error
handling function. getCcalls(from) is correctly below CSTACKMARK, since
we're in stack error handling, but from->nci is large (~1900) reflecting
the large number of active stack frames. I get that _unused_ CallInfos
are already accounted for in getCcalls (so that it need only be checked
when new CallInfos are allocated), but here its adding a number
reflecting the _used_ CallInfos, which is surely wrong. (L->nci is 0
because the coroutine has no CallInfos yet.)

So we get into the coroutine with a large value (>2000) in L->nCcalls,
and so the coroutine itself proceeds to recurse _another_ ~1900 times
before tripping its own nested stack error, at which point the whole
thing repeats. Hence the stack overflows.

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Roberto Ierusalimschy
> >>>>> "Yongheng" == Yongheng Chen <[hidden email]> writes:
>
>  Yongheng> The original POC that can crash a default build lua ( just
>  Yongheng> run make on Linux) is:
>
>  Yongheng> function errfunc() end function test()
>  Yongheng>     xpcall(test, coroutine.wrap(function() print(xpcall(test, errfunc)) end))
>  Yongheng>         end coroutine.wrap(function() print(xpcall(test, errfunc)) end)()
>
> So surely this, in lua_resume, can't possibly be right:
>
>         L->nCcalls = getCcalls(from) + from->nci - L->nci - CSTACKCF;
>
> We get here when the stack overflow error trips the first time and we
> try and resume the wrapped coroutine which is supplied as the error
> handling function. getCcalls(from) is correctly below CSTACKMARK, since
> we're in stack error handling, but from->nci is large (~1900) reflecting
> the large number of active stack frames. I get that _unused_ CallInfos
> are already accounted for in getCcalls (so that it need only be checked
> when new CallInfos are allocated), but here its adding a number
> reflecting the _used_ CallInfos, which is surely wrong. (L->nci is 0
> because the coroutine has no CallInfos yet.)

You are right (as usual :-). (A small detail: I think it is not adding
the number of used CallInfos, but the total number of CallInfos, used
plus unused.) A conservative fix would be not to add anything to
getCcalls(from).

-- Roberto
Reply | Threaded
Open this post in threaded view
|

RE: Stack overflow in luaO_pushvfstring

Yongheng Chen

Hi,

 

I tried the POC with the latest commit of lua and it seems the problem is gone. Has the problem been fixed? If so I can restart testing Lua.

 

Best,

Yongheng

 

From: [hidden email]
Sent: Friday, July 10, 2020 3:46 PM
To: [hidden email]
Subject: Re: Stack overflow in luaO_pushvfstring

 

> >>>>> "Yongheng" == Yongheng Chen <[hidden email]> writes:

>

>  Yongheng> The original POC that can crash a default build lua ( just

>  Yongheng> run make on Linux) is:

>

>  Yongheng> function errfunc() end function test()

>  Yongheng>     xpcall(test, coroutine.wrap(function() print(xpcall(test, errfunc)) end))

>  Yongheng>         end coroutine.wrap(function() print(xpcall(test, errfunc)) end)()

>

> So surely this, in lua_resume, can't possibly be right:

>

>         L->nCcalls = getCcalls(from) + from->nci - L->nci - CSTACKCF;

>

> We get here when the stack overflow error trips the first time and we

> try and resume the wrapped coroutine which is supplied as the error

> handling function. getCcalls(from) is correctly below CSTACKMARK, since

> we're in stack error handling, but from->nci is large (~1900) reflecting

> the large number of active stack frames. I get that _unused_ CallInfos

> are already accounted for in getCcalls (so that it need only be checked

> when new CallInfos are allocated), but here its adding a number

> reflecting the _used_ CallInfos, which is surely wrong. (L->nci is 0

> because the coroutine has no CallInfos yet.)

 

You are right (as usual :-). (A small detail: I think it is not adding

the number of used CallInfos, but the total number of CallInfos, used

plus unused.) A conservative fix would be not to add anything to

getCcalls(from).

 

-- Roberto

 

Reply | Threaded
Open this post in threaded view
|

Re: Stack overflow in luaO_pushvfstring

Roberto Ierusalimschy
>    I tried the POC with the latest commit of lua and it seems the problem is
>    gone. Has the problem been fixed? If so I can restart testing Lua.

"latest commit" is not very specific :-)

-- Roberto
Reply | Threaded
Open this post in threaded view
|

RE: Stack overflow in luaO_pushvfstring

Yongheng Chen

Oh sorry. I mean the commit

e1d8770f12542d34a3e32b825c95b93f8a341ee1, which is the latest one on GitHub.

 

Yongheng

Sent from Mail for Windows 10

 

From: [hidden email]
Sent: Wednesday, July 15, 2020 2:06 PM
To: [hidden email]
Subject: Re: Stack overflow in luaO_pushvfstring

 

>    I tried the POC with the latest commit of lua and it seems the problem is

>    gone. Has the problem been fixed? If so I can restart testing Lua.

 

"latest commit" is not very specific :-)

 

-- Roberto