The mode flag I would have missed

Between July 9 and August 20 this year, any registered user could get remote code execution on luarocks.org by uploading a package. LuaRocks disclosed it this weekend. The exploit window closed a month before anyone noticed.

The mechanism is small enough to fit in one paragraph. When someone uploads a rockspec, the server parses it inside a sandbox — setfenv on a fresh environment, no io, no os, no require. The parser calls loadstring on the uploaded text. loadstring returns a function. The function runs in the sandbox. If the uploaded text is Lua source, everything the sandbox forbids stays forbidden.

loadstring, on LuaJIT, also accepts precompiled bytecode. Bytecode does not go through the parser; it goes straight to the VM, and it can contain instructions that read and write memory outside the function’s own frame. The fix in the disclosure is one argument: pass "t" to loadstring, which restricts it to text mode and rejects any file whose first byte is \27 (the marker for a bytecode chunk). The sandbox stopped being a sandbox at the point where “the input is a program” stopped being true.

I have looked at this class of code many times. If a reviewer had put this diff in front of me — a sandboxed rockspec parser, setfenv set up correctly, loadstring called on the user’s input — I would have read it, checked the environment table, checked what the sandbox exposed, and approved it. I would not have looked up the second argument to loadstring. I have never looked up the second argument to loadstring. I would have missed it.

That is what bothers me about the bug. It is not a case where the author did something obviously careless and a careful reader would have caught it. The author reached for the standard Lua way to parse a string into a function, and the standard Lua way accepts one thing more than its name suggests. The reviewer would need to know, in advance, that this loader has modes, and that the default is the permissive one. If you have not been bitten by this before, “the loader has modes” is not a question you know to ask.

I run into this shape of thing constantly, in every language I read. eval in JavaScript takes a string; the string can be anything, including code that inspects the surrounding closure. pickle.load in Python takes bytes; the bytes can be anything, including instructions that call arbitrary constructors. yaml.load used to run tags that instantiated Python classes; yaml.safe_load is the same call with the mode argument you were supposed to know about. The pattern is: a loader named for the thing it usually loads, an API surface that accepts more than the name suggests, and a default that is the wider surface. Safety is the opt-in.

The remediation, if you read the disclosure, is not just the "t". LuaRocks revoked every API key, every session, every 2FA secret, and rebuilt the server on new infrastructure. That is what “we do not know what the attacker did while they were in” looks like in a security notice. The bug was one missing argument; the fallout was assuming that no state the server touched between July 9 and August 20 was trustworthy.

I do not have a rule to offer. “Look up the second argument to every loader you call” is either advice nobody follows or a full-time job. The thing I want to hold onto is smaller: when I read code that runs untrusted input through a function whose name promises safety, the question is not is this function safe, it is what does this function accept, and did the author say so explicitly. loadstring(src) and loadstring(src, "t") look almost the same. One of them is a sandbox and one of them is not, and the difference is the argument the author did not write.