Content Security Policy: avoid need for "unsafe-eval"?

[jsharkey13]

We use moo.js to parse mathematical answers provided by students in an online learning platform, to preview what they have typed. We're thus using it in-browser and are running into issues with our strict Content-Security-Policy blocking unsafe evaluation of JavsScript.

moo.js uses the unsafe Function constructor; apparently for speed reasons since switch-case is faster than attribute lookup (although the JSPerf link mentioned in that method no longer works for me?). Use of Function is blocked if a website uses a CSP header unless explicity enabled by using script-src: 'unsafe-eval'. We would strongly like to avoid this security vulnerability, but do not want to abandon this library.

At the moment we're just using a rewritten version of your keywordTransform method instead of the default one. Instead of using the switch-case built as a string and compiled down via Function, we just use the reverseMap object directly.
i.e. replacing the line:

return Function('value', source)

with

return function(k) {
    return reverseMap[k]
}

which seems to have exactly the same behaviour as the generated switch-case does, as far as I can tell.

In our use-case, with ~10 token types and 50 keywords, this seems to suffer about a 50% performance hit on using the switch-case Function version, but is still incredibly fast. Using our own version is not a particularly future-proof solution though.

It is actually possible to catch the CSP violation in a try-catch block, and alter the behaviour of the function; for example:

let functionConstructorAllowed;
try {
    Function();
    functionConstructorAllowed = true;
} catch {
    functionConstructorAllowed = false;
}

if (functionConstructorAllowed) {
    // ... existing behaviour using Function
} else {
    // ... alternative fallback above
}

Would you be happy to allow this library to fall back to an object attribute lookup, iff a CSP header was blocking the Function constructor? I could provide the pull request if so.
For users not blocking Function, this has only the performance overhead of one Function object creation and one if statement, only at initialisation when calling moo.keywords and otherwise behavior remains as-is. For users who do use a CSP; the library will be slower, but would then still function.

I would also be interested in knowing what the JSPerf example was, and how to benchmark the object attribute lookup fallback with a use-case other than ours.

[bd82]

Relevant references:

• Not working for web workers acornjs/acorn#90
• https://stackoverflow.com/questions/42458188/catching-content-security-policy-csp-errors

I am not sure you can rely on always being able to catch these type of exceptions.

[jsharkey13]

It works for me in Chrome, Firefox, Edge, Opera and Safari; and according to the references in that StackOverflow answer the CSP standard requires browsers "MUST" throw an EvalError on violation, which can be caught. So it will work in any standards-compliant browser; I think the only difference in behavior is whether it will also be reported as a violation. Since the alternative is this library not functioning, I don't think it matters too much?

Webpack uses the same process, though admittedly only in its templating code.

[bd82]

Since the alternative is this library not functioning

Not quite.
The alternative is the library functioning slower if the usage of new Function is removed. Or APIs for code generation instead of being a "pure runtime" library.

I don't think it matters too much?

That depends on the library author...

• Which browsers are targeted by this library (e.g mobile?)
• Should browser testing be implemented if CSP detection is used?
• Would the possible printing of a warning to the console impede adoption by corporate users?

Cheers.
Shahar.

[bbroereES]

PR created with the suggested change.

[tjvr]

With apologies for the late reply:

Would you be happy to allow this library to fall back to an object attribute lookup, iff a CSP header was blocking the Function constructor? I could provide the pull request if so.

That seems very reasonable to me! I would gladly review such a PR.

My only concern is that developers using Moo may not realise that this is happening, but I don't think there's a great way to fix that. We could throw in a console.warn but I imagine polluting the console in this way would be unpopular.

As far as benchmarks go, I'm afraid the JSPerf has been lost to time. If someone is able to demonstrate that the generated switch statement provides little benefit we can consider removing it, but from what you've said I don't think that is the case.

[nathan]

@tjvr

As far as benchmarks go, I'm afraid the JSPerf has been lost to time. If someone is able to demonstrate that the generated switch statement provides little benefit we can consider removing it, but from what you've said I don't think that is the case.

On arm64 macOS Chrome 99.

[jsharkey13]

@nathan
Am I correct in reading this as Map.get() being almost 10x faster than object attribute lookup, and 3x faster than the compiled switch statement? Maps seem widely supported by browsers now, and rewriting the code to use them would not be hard.

Do you have a link to the benchmark code? I could test it on Windows x64 too.

[tjvr]

Thanks for putting that together, @nathan!

I agree with what you wrote elsewhere:

Seems like Map is vastly faster, so we should probably use that and fall back to Object.create if the UA is old.

[tjvr]

Fixed in #173! Thanks to @jsharkey13 for writing and @nathan for reviewing 🙏