DEV Community

cursora
cursora

Posted on

vm.runInContext is not a sandbox: how we replaced eval with a parser

The feature

One of our challenge types lets a learner write a MongoDB query and run it against a seeded dataset. The obvious implementation, and the one we shipped first, is the one you're probably picturing: take the query string, run it in Node's vm module with a db object in the context, return the result.

vm.runInContext(userQuery, contextWithDb);
Enter fullscreen mode Exit fullscreen mode

To keep it read-only, there was an allowlist: a regex over the query string checking that the methods called were things like find, aggregate, countDocuments.

Both halves of that are wrong, and they're wrong in ways worth spelling out, because this exact shape shows up in a lot of "let users write a little expression" features.

Why the regex allowlist fails

A regex like \.(\w+)\( sees method calls written with dot notation. JavaScript does not require dot notation.

db.users.find({})["constructor"]["constructor"]("return process")()
Enter fullscreen mode Exit fullscreen mode

No .method( for the regex to object to, and constructor.constructor is Function — which builds a new function from a string. Once you can construct a function, the allowlist is decoration.

Any allowlist that operates on the text of a program rather than its structure has this class of hole. There is always another way to spell the same operation.

Why vm doesn't save you

The natural response is "fine, but it's running in a vm context, so it's contained." It isn't. Node's own documentation is explicit that the vm module is not a security mechanism — it isolates the global object, not the process. Escapes via constructor chains on objects passed into the context are well documented, and in our case an object had to be passed in: the db handle the whole feature exists to expose.

And that db handle was a live connection to the production database. So the ceiling on this bug was not "a learner reads another collection". It was arbitrary code execution in the backend process, holding a production database connection.

We found this in an internal audit in June, fixed it, and nothing indicates it was ever exercised in the wild — but "we found it before anyone else did" is luck, not architecture, and the architecture was the actual problem.

The fix: parse, don't evaluate

The rewrite executes no learner code at all. Not in a sandbox, not in a context, not anywhere.

The query string is parsed into an AST with acorn, and the AST is then interpreted against a strict allowlist of node types: literals, arrays, objects, a couple of permitted constructor calls (ObjectId(...), ISODate(...)), and a call chain matching db.<collection>.<method>(...). Anything else — a member expression that isn't in the shape we expect, an identifier we don't know, a function expression — is a rejection, not a fallback.

PRIMARY_READ_METHODS = find, findOne, aggregate, countDocuments, distinct
CHAIN_METHODS        = sort, limit, skip, project
WRITE_METHODS        = insertOne, updateOne, deleteMany, ... (author setup scripts only)
Enter fullscreen mode Exit fullscreen mode

The structural difference that matters: ["constructor"]["constructor"] is not a mysterious edge case to this design. It's a MemberExpression with a computed key, which isn't on the allowlist, so it's rejected — the same way any other unrecognized node is. The security property comes from the shape of what's permitted, not from enumerating what's forbidden.

Server-side JS operators ($where, $function, $accumulator) and cross-collection writes ($out, $merge) are separately blocked, because those move execution back to mongod where our parser has no say.

What generalizes

  • "Sandboxed eval" is usually a contradiction. If your threat model includes the user being hostile, the question isn't which sandbox — it's whether you can avoid evaluating their code at all. For anything expression-shaped, you usually can.
  • Never allowlist over source text. Allowlist over parsed structure. Text has infinite spellings; an AST has a finite node vocabulary.
  • Deny by default at the node level. "Unknown construct → reject" is a one-line invariant. "Known-bad construct → reject" is a list you will be maintaining against attackers forever.
  • Count what's reachable from inside, not just what's exposed. The regex was guarding the method names. The actual exposure was the live db handle sitting in scope next to them.

If you have a "users write a small expression" feature running through vm, eval, or new Function — that's the one. Go look at it.

Top comments (0)