BUG/MEDIUM: hlua: don't pass stale nargs argument to lua_resume()

In hlua_ctx_resume(), we call lua_resume() function like this:

  lua_resume(lua->T, hlua_states[lua->state_id], lua->nargs)

Once the call returns, we may call the function again with the same
hlua context when E_YIELD is returned (the execution was interrupted
and may be resumed through another lua_resume() call).

The 3rd argument to lua_resume(), 'nargs', is a hint passed to Lua to
know how many (optional) arguments were pushed on the stack prior to
resuming the execution (arguments that Lua will then expose to the Lua
script).

But here is the catch: we never reset lua->nargs between successive
lua_resume() calls, meaning that next lua_resume() calls will still
inherit from the initial nargs value that was set in hlua ctx prior
to calling hlua_ctx_resume() (our wrapper function) for the first time.

This is problematic, because despite not being explicitly mentioned in
the Lua documentation, passed arguments (to which `nargs` refer to), are
already consumed once lua_resume() returns.

This means that we cannot keep calling lua_resume() with non-zero nargs
if we don't push new arguments on the stack prior to resuming lua after
the initial call: nargs is proper to a single lua_resume() invocation.

Despite improper use of lua_resume() for a long time, this didn't cause
visible issues in the past with Lua 5.3, but it is particularly sensitive
starting with Lua 5.4.3 due to debugging hooks improvements that led to
some internal changes (see: lua/lua@58aa09a). Not using nargs properly
now exposes us to undefined behavior when resuming after a yield triggered
from a debugging hook, which may cause running scripts to crash
unexpectedly: for instance with Lua raising errors and complaining about
values being NULL where it should not be the case.

For reference, this issue was initially raised on the Lua mailing list:
  http://lua-users.org/lists/lua-l/2023-09/msg00005.html

In this patch, we immediately reset nargs when lua_resume() returns to
prevent any misuse.

It should be backported to every maintained versions.

diff --git a/src/hlua.c b/src/hlua.c
index e418b2d299ffa33a4ee8ca449c0c9adb72c20d13..b3c1b5da5b61349e7f5af125a423bfd9c329f488 100644 (file)
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1724,6 +1724,14 @@ resume_execution:
        /* out of lua processing, stop the timer */
        hlua_timer_stop(&lua->timer);
 
+       /* reset nargs because those possibly passed to the lua_resume() call
+        * were already consumed, and since we may call lua_resume() again
+        * after a successful yield, we don't want to pass stale nargs hint
+        * to the Lua API. As such, nargs should be set explicitly before each
+        * lua_resume() (or hlua_ctx_resume()) invocation if needed.
+        */
+       lua->nargs = 0;
+
        switch (ret) {
 
        case LUA_OK: