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.
(cherry picked from commit e7281f3f5d5b0b34e5c7e52ecb12255545255e1b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit fa76a10cad7dd814c414bc3d1e460b03ab55ca55)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 0880a74144005d4fe0d049b516aac266b1144427)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit b2e09aeaf157be094f939f524128e30d74691809)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
1 file changed