Instantsearch.js: "eval" in 2.6.3 requires the use of an unsafe-eval CSP directive

[borisschapira]

First of all, thank you for your solution which is really great and quite incredible in terms of performance!

Feature request

Remove the use of eval (found in …Function("return this")()||(0,eval)("this")… in https://cdn.jsdelivr.net/npm/[email protected]) so that we could use a script-src directive in our Content Securities Policies that would not be forced to unsafe-eval.

Use case

I have a pretty restrictive CSP on borisschapira.com and don't want to authorize all my third-party domains to execute eval in order to authorize Algolia Instant Search on my search page.

work to fix this (added by @haroenv)

remove or fork Hogan.js to remove the lines using new Function. I think this might be quite hard, since AFAICT, it's what allows {{{. We could enforce template strings maybe however 🤔

https://github.com/twitter/hogan.js/blob/7e340e9e4dde8faebd1ff34e62abc1c5dd8adb55/lib/compiler.js#L293

[Haroenv]

I guess this has been added by babel or webpack. Any chance you resolved a similar thing already?

[Haroenv]

full chunk responsible is:

/***/ }),
/* 38 */
/***/ (function(module, exports) {

var g;

// This works in non-strict mode
g = (function() {
    return this;
})();

try {
    // This works if eval is allowed (see CSP)
    g = g || Function("return this")() || (1,eval)("this");
} catch(e) {
    // This works if the window reference is available
    if(typeof window === "object")
        g = window;
}

// g can still be undefined, but nothing to do about it...
// We return undefined, instead of nothing here, so it's
// easier to handle this case. if(!global) { ...}

module.exports = g;

not sure yet which module it is

EDIT: I think it's a webpack handling of global usage

[Haroenv]

confirmed, the code in question is https://github.com/webpack/webpack/blob/fde018300aa52262c384e937c408d5dd97d62951/buildin/global.js#L10

https://github.com/webpack/webpack/issues/5627

[Haroenv]

@borisschapira note that you can still have a safe, non-breaking CSP with strict-dynamic which will work

[Haroenv]

This has been discussed in Webpack with the suggested solution of the strict-dynamic CSP directive, which is the recommended value. There's nothing we can really do except suggest a change in Webpack. If this issue is very important for you, I suggest you go into the Webpack threads and ask how you can help or how it can change there.

[borisschapira]

The 'strict-dynamic' source expression allows script loaded via nonce- or hash-based whitelists to load other script, simplifying the requirements for deployment.

I don't see how this could workaround the issue, insofar as it does not concern a prohibition to load a script from a domain, but a prohibition to execute it because of its content (an eval). The issue is still open, so this one should be too: https://github.com/webpack/webpack/issues/5627

I'm gonna try @torarnv's proposition, if I can understand how webpack configuration works…

Edit: Using webpack 4.1.1 with an explicit override of devtool: 'inline-source-map' to disable the default eval source-map for the development environment, I'm successfully able to produce an eval-less output for both production and development environments.

[borisschapira]

Ok, this would need to modify the Webpack version, and that's far beyond my reach :'(

[Haroenv]

If this is the case, then the eval will be gone in a future version when we upgrade Webpack. Not currently on the planning yet, since we had some incompatible plugins last we checked. I'll reopen so we remember to change to inline-source-map then. For now nothing can be done except changing the CSP

[borisschapira]

Just to be clear for future readers (or ourselves in several months), changing the CSP to accept the eval would mean adding unsafe-eval. In most cases, the common security recommendation is to avoid eval() at all costs, as it can be a vector for Cross-Site Scripting attacks (if you want to know more, learn about the script-src directive in a Content Security Policy).

[Spone]

I currently have a related issue with Instantsearch.js 2.10: Error: "call to Function() blocked by CSP"

It's coming from Hogan.makeTemplate https://github.com/twitter/hogan.js/blob/7e340e9e4dde8faebd1ff34e62abc1c5dd8adb55/lib/compiler.js#L293

I understand that Hogan is used internally by Instantsearch.js to handle templates. I should probably open an issue there. But is there a way to still use Instantsearch.js without Hogan in the meantime?

[Haroenv]

@spone, not directly, since templates are used internally too. What you can do is forking hogan (it’s not accepting PRs currently), making the change, and then we can backport it for the whole library, makes sense?

[Spone]

I'm not sure what is the change to make on Hogan.

new Function is used twice:

• template.code = new Function('c', 'p', 'i', this.wrapMain(codeObj.code));
• template.subs[key] = new Function('c', 'p', 't', 'i', codeObj.subs[key]);

But I don't see an easy way to eval the JS code safely in those cases...

[heiskr]

Hello hello 👋🏼 Sorry for bringing up an old issue. We're using instantsearch.js and we're wanting to remove the need for unsafe-eval in our content security policy headers. I'm looking through some issues and pull requests in this repository, and the status of Hogan for this project is a bit unclear to me. It looks like there's been attempts to replace Hogan with template literals or another mustache library. However, it looks like as of the current version, 4.9.0, that Hogan is still in place.

Should we consider this project to be permanently on Hogan and needing unsafe-eval indefinitely, or is replacement still possible? We'd be open to making a pull request to another templating option if time/effort is the blocker. I can understand though if it's too large of a breaking change for this project to consider.

Thanks :)

[Haroenv]

Thanks for your message @heiskr. For the moment it's too large of a breaking change for us to consider seriously, however if you find a good alternative that's:

• small(er than hogan)
• doesn't have unsafe evaluation
• compatible with IE11
• close enough API (conditionals, loops, helpers and interpolation is all we really need, partials etc. aren't needed)

We'd love a PR for this. Note that since it's so big, we can't guarantee that it'll be merged right away, but we'll keep it in close mind to either enable a drop-in replacement for the templating system (allowing yours to replace it, before we ship it as stable), or replacing it with a new API.

In the mean time you can alias hogan to a module with the same API, but which does much less as long as you only use the bare minimum of hogan should be all right. If you only use the connectors for example you won't even need it anymore at all.

If you're very interested in this, you can schedule a call with me ([email protected]) to hash out some details :)

[heiskr]

Awesome @Haroenv, nice to meet you! Thanks for the tip about aliasing, I totally forgot about it. I used webpack to alias out Hogan. I realized we're actually not using any of the templating built in as we do all our own templates, so I made a quick shim to fill Hogan so it doesn't error out.

I added this to our webpack config:

  resolve: {
    alias: {
      'hogan.js': path.resolve(__dirname, 'javascripts/fake-hogan.js')
    }
  }

And then my fake-hogan.js file is just:

export default {
  compile (template) {
    return {
      render (data) {
        return ''
      }
    }
  }
}

So that gets us to be able to remove unsafe-eval 🎉

---

While I was figuring this out, I looked briefly at some Mustache alternatives to Hogan.

| | Hogan | mustache.js | micromustache | Handlebars |
| -- | -- | -- | -- | -- |
| Size, minified non-zip | 9KB | 12KB | 6KB | 80KB |
| Eval | Yes | No | No | No |
| Interpolation | Yes | Yes | Yes | Yes |
| Conditionals/Loops | Yes | Yes | No | Yes |
| Helpers | Yes | No | No | Yes |

There are some API differences as well. micromustache is very similar. mustache.js doesn't have a compile step. Handlebars' compile returns a function and not an object with render on it.

[Haroenv]

micromustache looks very interesting, but due to it only doing string variable interpolation it might be a little too minimalistic for the use cases we have. Personally I've also evaluated these:

htm

• pro: has event listeners
• pro: is very small
• con: jsx isn't straightforward for those using only html
• con: doesn't work on IE11

nanohtml

• pro: is very small
• pro: has event listeners
• con: might not be very maintained
• con: doesn't work in IE11