search

mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.03k stars 1.24k forks source link

`hasEval` detection leads to CSP warnings and reports #11465

Closed tux21b closed 8 months ago

tux21b commented 8 months ago

Steps to reproduce

  1. Setup Content-Security-Policy that does not allow 'unsafe-eval' (like Content-Security-Policy: script-src 'self')
  2. Visit a site that imports "@mui/x-data-grid"

For example, run https://codesandbox.io/p/devbox/late-http-8nkhlj?file=%2Fnext.config.js%3A29%2C5 locally.

Current behavior

The hasEval detection added in https://github.com/mui/mui-x/issues/10056 is always automatically executed, even if the undocumented option disableEval is used. The hasEval detection runs an obfuscated version of eval("true") which gets blocked. This leads to CSP warnings reported via Report-To. The JavaScript exception is caught and hidden.

Expected behavior

I would prefer if eval() isn't used at all, but if that's not an option, then it should not be used if disableEval is set to true. Doing the computation of hasEval lazily (and only if disableEval is not set) would solve the problem.

Context

I'm trying to use the datagrid component in a secure way. Having Content-Security-Policy headers is highly recommended and disallowing arbitrary eval()s is the whole point of CSP. Having a report endpoint to catch possible security breaches (and errors within dev) is also recommended and having to continuously filter out false reports because of eval("true") within mui-x is really annoying and might hide more serious attacks or bugs.

Your environment

No response

Search keywords: datagrid csp eval

oliviertassinari commented 8 months ago

Does this connect back to https://github.com/mui/mui-x/pull/10329#discussion_r1327233747?

tux21b commented 8 months ago

Does this connect back to #10329 (comment)?

I don't think so. The Content-Security-Policy doesn't mind if the eval is obfuscated via atob or if it was executed in a global scope or if it was re-assigned to another variable. It just forbids the usage of eval (unless 'unsafe-eval' is explicitly set).

romgrk commented 8 months ago

Sounds good, we can evaluate hasEval lazily, so if disableEval is used it will never be evaluated.

flaviendelangle commented 8 months ago

even if the undocumented option disableEval is used

Not directly the topic here but this prop is probably worth documenting since we had several feedbacks of people wanting to use it. Having a clean doc section to link would make the process smoother

reihwald commented 8 months ago

I have the same issue as @tux21b and implemented a fix in #11516

oliviertassinari commented 8 months ago

I don't think so. The Content-Security-Policy doesn't mind if the eval is obfuscated via atob or if it was executed in a global scope or if it was re-assigned to another variable. It just forbids the usage of eval (unless 'unsafe-eval' is explicitly set).

@tux21b Ah right, without 'unsafe-eval' both eval and Function are blocked: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#unsafe_eval_expressions.