Remove the ability to load bytecode through load()

[lityge]

Hello everyone,
With a colleague of mine we have spent a few weeks reviewing how securely are mods implemented. We couldn't not take a look at Factorio, since it seems to be so well engineered
Loading untrusted bytecode is a huge issue in the Lua world, because there used to be a bytecode verifier to check that it was well-formed but it has been removed during the transition from Lua 5.1 to Lua 5.2. It such a big issue, that in 2012 the developers of Roblox have decided to remove the ability to load bytecode from their modding interface [0].
We have tried to exploit the Lua implementation within Factorio using well known Lua tricks (almost a decade old for some of them), however we were not successful (mostly because our lack of time). However theoretically it remains possible to exploit the Lua engine within Factorio to escalate privileges and run arbitrary code through modding.

Would you consider removing the load() function ?

We have documented our attempts on github [1].



[0] : https://blog.roblox.com/2012/08/bye-bye-bytecode/
[1] : https://github.com/gbip/lua_attack

[Klonan]

however we were not successful (mostly because our lack of time).

Well if you are ever successful, let us know and we can try to patch the Lua source,
We have already added a few bytecode checks back into the Lua source, and we are happy to add more to keep it safe

But removing the load function just because hypothetically someone can do bad with it, isn't the right choice

[lityge]

Is there that much use within the modding scene of factorio of the load() function ?
We have been able to create Lua primitives that allows memory read/write at arbitrary addresses within the Factorio address space. Achieving native code execution require further work, but is definitively possible.

[Klonan]

We have been able to create Lua primitives that allows memory read/write at arbitrary addresses within the Factorio address space. Achieving native code execution require further work, but is definitively possible.

What version of the game are you using?
If you can share your work we can fix the problems in the Lua source code

[lityge]

We were working on Factorio 0.17.79-0, which is based on Lua 5.2.1 if I am not mistaken.
We have documented our findings and our attempts in this github repository : https://github.com/gbip/lua_attack

[Klonan]

We were working on Factorio 0.17.79-0, which is based on Lua 5.2.1 if I am not mistaken.
We have documented our findings and our attempts in this github repository : https://github.com/gbip/lua_attack

Can you try using the latest 0.18 experimental?
We have patched several exploits in the last few weeks

[posila]

I don't know if you are the same person who emailed us a couple of weeks ago ... if so, I guess nobody responded to you (sorry).

Would you consider removing the load() function ?

There is one project in particular, developed by user justrandomgeek, that is counting on load(), so we tried to patch up bytecode execution for 0.18.19. Namely OP_SETLIST and OP_FORLOOP check type of their parameters, so the particular technique you used will not be possible anymore ... some patches were applied also to OP_TFORCALL, OP_TFORLOOP and OP_EXTRAARG.

If you have other ideas how to breach the sandbox, please let us know. It's possible it's fools errand to try to patch the bytecode execution.

[lityge]

I don't know if you are the same person who emailed us a couple of weeks ago ... if so, I guess nobody responded to you (sorry).

Would you consider removing the load() function ?

There is one project in particular, developed by user justrandomgeek, that is counting on load(), so we tried to patch up bytecode execution for 0.18.19. Namely OP_SETLIST and OP_FORLOOP, so the particular technique you used will not be possible anymore ... some patches were applied also to OP_TFORCALL, OP_TFORLOOP and OP_EXTRAARG.

If you have other ideas how to breach the sandbox, please let us know. It's possible it's fools errand to try to patch the bytecode execution.

We have not emailed you, so maybe someone else has been working on the same subject ? I guess you patched the vulnerability used for this specific exploit.

[aaron311]

I don't know if you are the same person who emailed us a couple of weeks ago ... if so, I guess nobody responded to you (sorry).

Hey ... I was the one who emailed, and yes, you folks *did* respond to me (thanks!). Since the cat's out of the bag now, I guess it sort-of no longer does any harm to comment here? (Been avoiding that so far to keep things under wraps.)

Glad to see you patched the specific vectors I was exploring. To be honest it took me forever to even get my first attempt at a partial escape working, so I probably won't go back looking for more vectors nor will I verify the fixes. I'll leave that to the more experienced folks.

As I said in the email, and as the OP said, I'm impressed by how well you locked this thing down in the first place. Lua seems like it's not really originally designed for sandboxing very well, and yet you folks had blocked the obvious vectors I tried.

If you have other ideas how to breach the sandbox, please let us know. It's possible it's fools errand to try to patch the bytecode execution.

I share the same concern. Did you see my latest email (from a week ago) regarding an alternate suggestion for this? Namely this:

If disabling raw bytecode loading in the Lua mod scripts is indeed infeasible for whatever reason, you might consider whether a different restriction could have the same effect. For instance, if you enforced a new rule of 'we will only load bytecode chunks that we have previously produced in the same Lua VM execution' I think it also closes off the vulnerability entirely since there's no way (to my knowledge) to convince the Lua script compiler to produce such invalid sequences.

For instance, generate a 256-bit HMAC key at Factorio start. Anything native method dumping a function to bytecode could 'sign' that with HMAC-sha256 and tack the auth tag produced on to the end of the bytecode blob. Anything loading bytecode blob would re-HMAC the input and compare the computed results with the ending tag. At first glance, a 'trick' like this would break your VM's determinism since the auth tag would be different per machine, but perhaps the HMAC tag could be a hidden field in the TString variable.

Or maybe you just keep a table in memory of the SHA-256 hashes of all 'trusted' bytecode chunks you've produced during this process's lifetime and compare any loaded blob to that before proceeding to load it.

The reason I suggest this is that I think it will be rather difficult to prevent all possible raw-bytecode attacks given the (rather poor) assumptions made by the current Lua VM implementation. It just doesn't seemed designed with security against bytecode attacks in mind. Maybe you can fix it manually but it seems like a huge job...

[lityge]

To be honest it took me forever to even get my first attempt at a partial escape working, so I probably won't go back looking for more vectors nor will I verify the fixes. I'll leave that to the more experienced folks.

What did your partial escape allowed you to do ? If you can send me the technical details I am very curious !
We have also spent quite a bit of time crafting this exploit (more or less a month, although we worked on other things as well), so we probably won't spend more time discovering other Lua exploits as we don't have anymore the time, nor the resources to work on this.

[aaron311]

What did your partial escape allowed you to do ? If you can send me the technical details I am very curious !

Hey, happy (and even eager) to share, but in my initial email note about the issue to the Factorio devs I promised to keep the details under wraps. So I'm gonna keep to that promise unless I'm told I'm good to share what I came up with.

(Probably they'll want to wait until we think the issue is entirely mitigated and enough people have upgraded?)

[posila]

What did your partial escape allowed you to do ? If you can send me the technical details I am very curious !

Hey, happy (and even eager) to share, but in my initial email note about the issue to the Factorio devs I promised to keep the details under wraps. So I'm gonna keep to that promise unless I'm told I'm good to share what I came up with.

(Probably they'll want to wait until we think the issue is entirely mitigated and enough people have upgraded?)

Feel free to share with lityge through private messages.

Thank you for the email, by the way.

[justarandomgeek]

Aaron, the problem with restricting to bytecode produced internally is that i'm currently working on a project that is essentially a standalone compiler for a slightly extended language (things like ?. and += and type annotations in source rather than comments) that will ideally compile to lua52/factorio compatible bytecode (by assembling bytes from scratch), so I'm pushing pretty hard to keep the ability to load externally produced bytecode. Thus, I'm happy to chase and patch the VM bugs if that means I can keep `load()`. I'm also happy to identify my bytecode somehow as "less suspicious" if somebody can come up with a way that isn't immediately abusable to mark evil things the same way. If I *really* have to I'll give up a few of the optimizations and generate lua text instead, but that also loses direct mapping in the debug symbols, meaning I have to implement another layer of mapping on top of those, and have less control over placing the stepping cursor correctly.

[aaron311]

@justarandomgeek

Ahhhh, that makes sense. I can see the argument. Realistically, though, I'm just not sure the Lua VM interpreter can be locked down sufficiently well. The interpreter code just doesn't seem designed with that in mind. Then again, if anyone is to be successful at securing the interpreter, I *would* bet on the Factorio devs being the ones!

If you guys do make enhancements to the interpreter to secure against bad bytecode, you might want to seriously consider contributing them back to Lua. I could see a TON of other games wanting that.


Edit: Another crazy idea: what if certain mods could be granted special 'bytecode' permission? But most would not be?

Or, maybe just include your new compiler in the game natively lol. Sorry, I'll shut up now.

[aaron311]

Feel free to share with lityge through private messages.

Okay, will do. Expect a PM this evening @lityge.

Thank you for the email, by the way.

NP. Thank you for the great game! The factory must grow!

[justarandomgeek]

I also happen to be spending a lot of time in the VM's guts along the way, so I'm looking for other unchecked assumptions that need testing as I go too, but most of the ones I've found so far are pretty boring (posila mentioned a few of the ones that seemed interesting and got some work). We also luck out in a few other cases by having coroutines disabled entirely we can strictly enforce some pairings that couldn't be if coroutines were enabled and avoid a whole category of abuse.

[aaron311]

I also happen to be spending a lot of time in the VM's guts along the way, so I'm looking for other unchecked assumptions that need testing as I go too, but most of the ones I've found so far are pretty boring (posila mentioned a few of the ones that seemed interesting and got some work). We also luck out in a few other cases by having coroutines disabled entirely we can strictly enforce some pairings that couldn't be if coroutines were enabled and avoid a whole category of abuse.

Niiice. Sounds like locking down the raw bytecode-processing might be within reach. If you're successful you all should really write a 'how to' guide for game developers to follow when locking down / sandboxing Lua. Of all the games that allow Lua-based scripting, my guess is that Factorio is already one of the least vulnerable.

As you noted, removing Coroutines is probably somewhere close to step 1.

[Rseding91]

A small note: the way the C version of Lua compiled as C++ implements coroutine yield is absolutely terrible: it throws an exception and catches it up at the previous call site.