Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
makavelithadon opened this issue Sep 8, 2023 · 7 comments

Comments

@makavelithadon
Copy link

makavelithadon commented Sep 8, 2023

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):

  • OS: Ubuntu 22.04.3 LTS
  • MJML Version 4.14.1
  • MJML Browser Version 4.14.1

Additional context
Saw this issue - webpack/webpack#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
Copy link
Member

iRyusa commented Sep 8, 2023

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
Copy link
Author

makavelithadon commented Sep 8, 2023

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 webpack/webpack#6461 (comment) 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
Copy link
Member

iRyusa commented Sep 8, 2023

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
Copy link
Author

+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 👍

@makavelithadon
Copy link
Author

makavelithadon commented Sep 11, 2023

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 :

  • Upgrade the webpack version to use a more recent one that do not use the eval-like code
  • Upgrade all top-level packages that embed the same code in their bundle

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
Copy link
Member

iRyusa commented Sep 15, 2023

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
Copy link

mvtango commented Sep 5, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants