On 2011/2/13, Mark de Wever wrote:
> The commit log of revision 48229 reads:
>
> <quote>
> Reverted part of r48216, since it opened a major security breach in
> Wesnoth. Indeed, it caused the Lua engine to skip unwinding of its state
> in presence of an exception, which opened the way to several attacks.
> For instance, setting gc finalizers and then forcing GUI2 to throw an
> exception would allow a multiplayer scenario to execute arbitrary code
> on a remote client.
> </quote>
>
> I wonder about two things:
>
> - how can somebody set a gc finalizer? Can this done by a lua code
>  itself or only from C++?

It can be done by Lua code (e.g. setmetatable(..., { __gc = ... })).

And even if it wasn't possible, there are still hundreds of finalizers
set by the C++ code (units, WML handlers, and so on) that are still
executed at scenario end. They require the Lua interpreter to be in a
sensible state, otherwise you would get all kind of memory corruption.

> - if it is a security issue when an exception is thrown, then why is it
>  save if the user terminates Wesnoth normally?

Why do you think it is safe? There isn't a single exception that goes
past LUAI_TRY! Its code was

try { something that might throw }
catch (...) { something that doesn't throw }

Note that the "..." is not an ellipsis for the sake of concision. It
is really the catch-all construct of the C++ language. And you decided
to change the code to

try { something that might throw }
catch (twml_exception&) { throw; } /* !!! */
catch (...) { something that doesn't throw }

What made you think that modifying the upstream Lua interpreter would
have no adverse affect? Did you send an email to the Lua devel
mailing-list and ask "Hi guys, I'm adding a jump that arbitrarily
skips hundreds of lines of C code in the Lua engine, is that safe?" ?
Somehow it's probably better you didn't. Otherwise you would have got
replies like "Are you an idiot?!".

So I repeat: it isn't safe to throw exceptions and thus skip the
cleanup code of the Lua engine. And guess what? Wesnoth wasn't doing
it, until you came. Before you start messing with upstream code,
please fix all the bugs you have been introducing with GUI2!

_______________________________________________
Wesnoth-dev mailing list
[email protected]
https://mail.gna.org/listinfo/wesnoth-dev

Reply via email to