Is it safe to use the `Function` constructor to validate JavaScript syntax?

Question

I would like to verify (client-side) that the user has entered valid JavaScript code.

Pulling in a Javascript parser (e.g. Acorn or Esprima) is a relatively heavy dependency. However (if CSP is not enabled), I could do:

try {
    new Function(userCode);
    return {valid: true}
} catch (e) {
    return {
        valid: false,
        error: e.message
    };
}

While this does use new Function, the user-provided code is never run, only parsed/compiled. Is there still a security issue?

Answer 1

This seems fine. Parsing/compiling JS code is safe and has no side effects (module resolution and loading does not apply in Function). We can be certain that JavaScript parsers used in modern browsers are pretty free of bugs.

Answer 2

The standard (even as early as ECMA-262 1st Ed., §15.3.2.1, and all later ones too) specifies that invoking the Function constructor will only parse and compile the code it is given, not execute it; so on paper, and in modern, mainstream-browser engines, this should probably be fine. But even under their relatively recent versions (those available as late as 2016), this actually used to be quite unsafe. Some engines used to implement the Function constructor as the moral equivalent of:

function Function() {
  var args = Array.prototype.slice.call(arguments, 0, -1).join(",");
  var body = arguments[arguments.length - 1];
  return (0, eval)("(function anonymous(" + args + "\n) {\n" + body + "\n});");
}

with all the consequences – in particular, under V8 you could write something like new Function("}, alert(1), function () {") and have it execute arbitrary code: see bug #2470. Meanwhile, JavaScriptCore could outright crash when fed invalid code to the constructor: see bugs #106160 or #163748. Less mainstream engines may be prone to making the same shortcut even today. For example, QuickJS does so, and is still vulnerable to this trick as of 2024: https://github.com/bellard/quickjs/blob/06651314f5bbef1bb7a2689da7fd7b2b04ce6125/quickjs.c#L38342.

To be safe against such sloppy implementations, you may want to check for known bugs in the Function constructor and disable client-side checks if any vulnerabilities are detected.

function isSyntaxCheckingSafe() {
  function check() {
    try {
      Function.apply(null, arguments);
      return true;
    } catch (e) {
      if (!(e instanceof SyntaxError))
        return true;
    }
    return false;
  }

  /* this one first, as it does not outright crash buggy JavaScriptCore */
  if (check("/*", "*/){"))
    return false;
  if (check("},function(){"))
    return false;
  if (check("};{"))
    return false;

  /* no problems found, fingers crossed */
  return true;
}

console.log(isSyntaxCheckingSafe());

The above tests are prone to generating false-ish positives in that they will judge engines that successfully parse malformed Function arguments, but do not actually execute any injected code to be vulnerable. But better safe than sorry.

---

Note also that there are limitations to this approach other than safety:

• Since the Function constructor creates synchronous function objects, await expressions will be rejected. If you intend to accept asynchronous code, this can be remedied by using (async function () {}).constructor instead of Function.
• Code using the return statement will always be accepted, even if you intend to later insert it in a non-function context. There is no work-around for this.
• Module directives like import will be rejected. There is no work-around for this either; one may appear in the future if the Compartments proposal is accepted, which as of this writing (March 2024) is at stage 1 and includes a ModuleSource constructor that can parse module code.
• The environment where you validate the code may differ from the one where you eventually use it: a browser engine may support syntax constructs your (one would hope) sandboxed server environment does not, and vice versa.

Given that, such a test should probably be considered a best-effort sanity check at best (just like every other client-side check, really) that will only offload typos. You should keep independent server-side checks in place, not assume any client-side checks were performed, and even disable client-side checks if the environment available there proves incapable (because of subpar security or missing features).