mjmlio / mjml

MJML: the only framework that makes responsive-email easy
https://mjml.io
MIT License
17.07k stars 962 forks source link

mjml-browser generated bundle uses some 'eval' code that causes CSP 'unsafe-eval' issue #2742

Open makavelithadon opened 1 year ago

makavelithadon commented 1 year ago

Describe the bug First of all thanks for the incredible work realized !

We use the mjml-browser package to make some basic mjml->html conversion but the mjml-browser bundle contains some code that causes CSP (Content Security Policy) issues:

Screenshot from 2023-09-08 15-56-35

This issue is due to code that is equivalent to use of eval (cf. https://developer.mozilla.org/fr/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#unsafe-eval)

In the bundle file there is at least two occurrences of eval code :

Screenshot from 2023-09-08 15-57-28

And

Screenshot from 2023-09-08 15-57-41

To Reproduce Steps to reproduce the behavior:

  1. Create a website with CSP and minimal whitelist rules (for example 'strict-dynamic' and 'self' for script-src CSP policy)
  2. require mjml-browser in your code
  3. See error :

Screenshot from 2023-09-08 15-56-35

Expected behavior No use of unsafe-eval in production code

MJML environment (please complete the following information):

Additional context Saw this issue - https://github.com/webpack/webpack/issues/6461 - so applied some recommended potential solutions, but I was only able to remove the first "eval" code (the call to new Function("return this")) but not the second one (new Function(""+e)).

Maybe an upgrade to a more recent version of webpack can help (tried upgrading to webpack@latest but too much errors at build time)

It's very crucial for us to keep consistent CSP rules to avoid security issues and to stay aligned with best practices in web security.

Thanks in advance for your help

iRyusa commented 1 year ago

I wont have time to debug this one anytime soon as browser build is just a "POC". It comes most likely from a direct dependency like Juice maybe.

makavelithadon commented 1 year ago

After some research I think it comes from the setimmediate package from global node_modules.

cf. https://github.com/YuzuJS/setImmediate

and the responsible code https://github.com/YuzuJS/setImmediate/blob/master/setImmediate.js#L17

I re-tried the solution provided here https://github.com/webpack/webpack/issues/6461#issuecomment-394318550 and I needed to add this one too :

node: {
  setImmediate: false
}

Now the bundle no longer contains any reference to new Function(""...)...

In a nutshell : webpack.config.js:

{
  ...
  import webpack from "webpack"
  ...
  node: {
    global: false,
    setImmediate: false
  },
  plugins: [
    ...
    new webpack.ProvidePlugin({
      global: require.resolve('./global.js')
    })
    ...
  ],
  ...
}

global.js:

module.exports = window

After a new fresh build, no "eval" code anymore.

Is it acceptable ?

I do not know if you can try it from your side or if a PR can be submitted to implement these minor changes?

Thanks in advance

iRyusa commented 1 year ago

Looks like to be a dep of webpack... it may be better to try with a more recent bundler as we're still using webpack 4... I think you can open a pr about the webpack config in mjml-browser package

makavelithadon commented 1 year ago

+1 for upgrade to a more recent bundler ^^

So I will prepare a PR about webpack config in the concerned package and submit it as soon as possible.

Tahnks for your reactivity :+1:

makavelithadon commented 1 year ago

Hi,

Apparently it is no sufficient to get rid of the issue.

I managed to remove the two occurrences of eval-like code listed above (new Function("return this") and new Function(""+e)), but there's still another eval-like code that I did not manage to remove (Function("return this") without the new keyword).

Even if I explicitly mark node.global to false in webpack.config.js and I provide a custom global value (like window) through the ProvidePlugin webpack plugin...

After some search in the project at the global level, it seems that it has many packages whose bundle uses an eval-like code (like lodash, es6-promise, handlebars, json5, parser-html, parser-flow, parser-babel...).

So there are at least 2 subjects in my opinion :

I did not have the time to test with another bundler like Rollup or Parcel but it appears that it will be a bit complex to address this issue and for sure not effortless...

iRyusa commented 1 year ago

Yeah I think mjml-browser need to use a more "modern" bundler. I don't have time to test but I don't think something like esbuild would require much config to have a bundled version ?

mvtango commented 2 months ago

Good morning and thanks for the amazing work on MJML. I subscribed to this issue because it would solve a problem for me. OTOH, MJML already solves so many problems that this one in particular does not seem to be important in comparison.