> On 12 Sep 2021, at 08:21, Willy Tarreau <[email protected]> wrote: > > On Sat, Sep 11, 2021 at 11:17:25PM +0200, Tim Duesterhus wrote: >> In one case before exiting leaving the function the panic handler was not >> reset. >> >> Introduced in 69c581a09271e91d306e7b9080502a36abdc415e, which is 2.5+. >> No backport required. > > Good catch, applied as medium since it seems none of us can clearly > predict the effect :-)
Good question :-) This system manage longjmp() calls used to catch Lua errors. The longjmp() is set through the function lua_atpanic() before executing any Lua code. The haproxy lua panic hanlder call a longjmp which return to the lua execution caller and an error is processed. When the Lua code execution is done (with or without error) reseting this panic function restore the default Lua behavior which is an abort(). see https://www.lua.org/manual/5.3/manual.html#4.6 So, when the longjmp is not reset, if some Lua code is executed ommiting to set the longjmp (function lua_atpanic() crush the precedent registered panic function) and the Lua code raises an exception a longjmp is processed in a point of stack which contains random data. In the best case, we have a segfault, in a worst case Haproxy continne running doing crap. The same case (Lua code is executed ommiting to set the longjmp and the Lua code raises an exception) with the default panic handler throws an abort(). A buffer overflow could be exploited only if haproxy try to execute Lua code without setting the safe environment. I hope this is not the case. I’m confident because if some lus code were executed without the panic handler, abort() were already observed for a long time. So, I guess this ommit is not a great bug, but the experieence learn when we play with longjmp, MEDIUM is the right level for a patch. Thierry
