From 842a83f09caa2ebd4bc03e0076420148ac07c808 Mon Sep 17 00:00:00 2001 From: Roberto Ierusalimschy Date: Fri, 24 Nov 2023 16:08:55 -0300 Subject: Panic functions should not raise errors The standard panic function was using 'lua_tostring', which may raise a memory-allocation error if error value is a number. --- lauxlib.c | 9 +++++++-- ltests.c | 5 +++-- manual/manual.of | 4 ++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lauxlib.c b/lauxlib.c index 4ca6c654..1c9082e6 100644 --- a/lauxlib.c +++ b/lauxlib.c @@ -1025,9 +1025,14 @@ static void *l_alloc (void *ud, void *ptr, size_t osize, size_t nsize) { } +/* +** Standard panic funcion just prints an error message. The test +** with 'lua_type' avoids possible memory errors in 'lua_tostring'. +*/ static int panic (lua_State *L) { - const char *msg = lua_tostring(L, -1); - if (msg == NULL) msg = "error object is not a string"; + const char *msg = (lua_type(L, -1) == LUA_TSTRING) + ? lua_tostring(L, -1) + : "error object is not a string"; lua_writestringerror("PANIC: unprotected error in call to Lua API (%s)\n", msg); return 0; /* return to Lua to abort */ diff --git a/ltests.c b/ltests.c index 7d184c0d..c2943a4f 100644 --- a/ltests.c +++ b/ltests.c @@ -73,8 +73,9 @@ static void badexit (const char *fmt, const char *s1, const char *s2) { static int tpanic (lua_State *L) { - const char *msg = lua_tostring(L, -1); - if (msg == NULL) msg = "error object is not a string"; + const char *msg = (lua_type(L, -1) == LUA_TSTRING) + ? lua_tostring(L, -1) + : "error object is not a string"; return (badexit("PANIC: unprotected error in call to Lua API (%s)\n", msg, NULL), 0); /* do not return to Lua */ diff --git a/manual/manual.of b/manual/manual.of index ad120f5e..cef3e22a 100644 --- a/manual/manual.of +++ b/manual/manual.of @@ -4486,6 +4486,10 @@ This string always has a zero (@Char{\0}) after its last character (as @N{in C}), but can contain other zeros in its body. +This function can raise memory errors only +when converting a number to a string +(as then it has to create a new string). + } @APIEntry{lua_Number lua_tonumber (lua_State *L, int index);| -- cgit v1.2.3-55-g6feb From 5853c37a83ec66ccb45094f9aeac23dfdbcde671 Mon Sep 17 00:00:00 2001 From: Roberto Ierusalimschy Date: Thu, 21 Dec 2023 13:37:51 -0300 Subject: Bug: Buffer overflow in string concatenation Even if the string fits in size_t, the whole size of the TString object can overflow when we add the header. --- lstring.c | 2 +- lvm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lstring.c b/lstring.c index e921dd0f..97757355 100644 --- a/lstring.c +++ b/lstring.c @@ -224,7 +224,7 @@ TString *luaS_newlstr (lua_State *L, const char *str, size_t l) { return internshrstr(L, str, l); else { TString *ts; - if (l_unlikely(l >= (MAX_SIZE - sizeof(TString))/sizeof(char))) + if (l_unlikely(l * sizeof(char) >= (MAX_SIZE - sizeof(TString)))) luaM_toobig(L); ts = luaS_createlngstrobj(L, l); memcpy(getlngstr(ts), str, l * sizeof(char)); diff --git a/lvm.c b/lvm.c index 4d71cfff..918ae64c 100644 --- a/lvm.c +++ b/lvm.c @@ -661,7 +661,7 @@ void luaV_concat (lua_State *L, int total) { /* collect total length and number of strings */ for (n = 1; n < total && tostring(L, s2v(top - n - 1)); n++) { size_t l = tsslen(tsvalue(s2v(top - n - 1))); - if (l_unlikely(l >= (MAX_SIZE/sizeof(char)) - tl)) { + if (l_unlikely(l >= MAX_SIZE - sizeof(TString) - tl)) { L->top.p = top - total; /* pop strings to avoid wasting stack */ luaG_runerror(L, "string length overflow"); } -- cgit v1.2.3-55-g6feb From e288c5a91883793d14ed9e9d93464f6ee0b08915 Mon Sep 17 00:00:00 2001 From: Roberto Ierusalimschy Date: Thu, 11 Jan 2024 13:44:16 -0300 Subject: Bug: Yielding in a hook stops in the wrong instruction Yielding in a hook must decrease the program counter, because it already counted an instruction that, in the end, was not executed. However, that decrement should be done only when about to restart the thread. Otherwise, inspecting the thread with the debug library shows it one instruction behind of where it really is. --- ldebug.c | 5 ++--- ldo.c | 4 ++++ testes/coroutine.lua | 8 +++++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/ldebug.c b/ldebug.c index b1f16ac9..d6f132ea 100644 --- a/ldebug.c +++ b/ldebug.c @@ -925,12 +925,12 @@ int luaG_traceexec (lua_State *L, const Instruction *pc) { } pc++; /* reference is always next instruction */ ci->u.l.savedpc = pc; /* save 'pc' */ - counthook = (--L->hookcount == 0 && (mask & LUA_MASKCOUNT)); + counthook = (mask & LUA_MASKCOUNT) && (--L->hookcount == 0); if (counthook) resethookcount(L); /* reset count */ else if (!(mask & LUA_MASKLINE)) return 1; /* no line hook and count != 0; nothing to be done now */ - if (ci->callstatus & CIST_HOOKYIELD) { /* called hook last time? */ + if (ci->callstatus & CIST_HOOKYIELD) { /* hook yielded last time? */ ci->callstatus &= ~CIST_HOOKYIELD; /* erase mark */ return 1; /* do not call hook again (VM yielded, so it did not move) */ } @@ -952,7 +952,6 @@ int luaG_traceexec (lua_State *L, const Instruction *pc) { if (L->status == LUA_YIELD) { /* did hook yield? */ if (counthook) L->hookcount = 1; /* undo decrement to zero */ - ci->u.l.savedpc--; /* undo increment (resume will increment it again) */ ci->callstatus |= CIST_HOOKYIELD; /* mark that it yielded */ luaD_throw(L, LUA_YIELD); } diff --git a/ldo.c b/ldo.c index bd8d965f..ea052950 100644 --- a/ldo.c +++ b/ldo.c @@ -792,6 +792,10 @@ static void resume (lua_State *L, void *ud) { lua_assert(L->status == LUA_YIELD); L->status = LUA_OK; /* mark that it is running (again) */ if (isLua(ci)) { /* yielded inside a hook? */ + /* undo increment made by 'luaG_traceexec': instruction was not + executed yet */ + lua_assert(ci->callstatus & CIST_HOOKYIELD); + ci->u.l.savedpc--; L->top.p = firstArg; /* discard arguments */ luaV_execute(L, ci); /* just continue running Lua code */ } diff --git a/testes/coroutine.lua b/testes/coroutine.lua index de7e46fb..e566c86e 100644 --- a/testes/coroutine.lua +++ b/testes/coroutine.lua @@ -610,18 +610,20 @@ else -- (bug in 5.2/5.3) c = coroutine.create(function (a, ...) T.sethook("yield 0", "l") -- will yield on next two lines - assert(a == 10) + local b = a return ... end) assert(coroutine.resume(c, 1, 2, 3)) -- start coroutine local n,v = debug.getlocal(c, 0, 1) -- check its local - assert(n == "a" and v == 1) + assert(n == "a" and v == 1 and debug.getlocal(c, 0, 2) ~= "b") assert(debug.setlocal(c, 0, 1, 10)) -- test 'setlocal' local t = debug.getinfo(c, 0) -- test 'getinfo' - assert(t.currentline == t.linedefined + 1) + assert(t.currentline == t.linedefined + 2) assert(not debug.getinfo(c, 1)) -- no other level assert(coroutine.resume(c)) -- run next line + local n,v = debug.getlocal(c, 0, 2) -- check next local + assert(n == "b" and v == 10) v = {coroutine.resume(c)} -- finish coroutine assert(v[1] == true and v[2] == 2 and v[3] == 3 and v[4] == undef) assert(not coroutine.resume(c)) -- cgit v1.2.3-55-g6feb