> On 12 Sep 2021, at 08:21, Willy Tarreau 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
