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

dump() -> CSP-violation - (directive 'style-src') #49068

Open
oioix opened this issue Jan 22, 2023 · 11 comments
Open

dump() -> CSP-violation - (directive 'style-src') #49068

oioix opened this issue Jan 22, 2023 · 11 comments

Comments

@oioix
Copy link

oioix commented Jan 22, 2023

Symfony version(s) affected

current

Description

dump() is perfect for debugging. The dump window is apparently generated via a JS <script> Sfdump = window.Sfdump ... .
The use of CSP to protect the website against XSS is now becoming more and more important.
After switching to a stricter CSP directive that works with a nonce and will no longer allows unsafe-inline for security reasons, there are now (at every page load) two CSP violations occuring when using dump() which (in my configuration) leads for every single violation in sending an CSP-violation-report-e-mail and and an entry in the CSP-violation-log.

The tags <script> and <style> can easily be supplemented with the required nonce via str_replace() and are therefore absolutely no problem with the use of a CSP-nonce.
But inline css like e.g. <div style="display:none"> or javascript like onclick="alert('Hey')", which is used within a tag, unfortunately cannot be legitimated with a nonce an will lead to CSP-violation as soon unsafe-inline is no longer allowed. And that's exactly the problem that I herewith report about.

When using dump() the CSP-violation report looks like this:
Screenshot 2023-01-09 | 22 55 00

I checked the locations 554 and 691 named in the CSP report for potentially critical content. Both places contain this:
Screenshot 2023-01-11 | 22 16 55
(Note that the locations 554 and 691 may vary because of the nonce I use in the <script> and <style> tag.

It looks like a refStyle with a { display: none; } is appended via an innerHTML (append) .

The CSP report outputs a sample at position 554 (see also the red image above):
Source: pre.sf-dump .sf-dump-compact, .sf-dump-str-collapse .sf-dump-str-collapse, .sf-dump-str-expand .sf-dump-str-expand { display: none; }

And this exactly matches the JS code in the pages html-output:
refStyle.innerHTML = 'pre.sf-dump .sf-dump-compact, .sf-dump-str-collapse .sf-dump-str-collapse, .sf-dump-str-expand .sf-dump-str-expand { display: none; }';

How to reproduce

This problem can be reproduced using a CSP directive like:

Content-Security-Policy-Report-Only: script-src 'self' 'nonce-123456789' 'report-sample'; style-src 'self' 'nonce-'123456789' 'report-sample'; report-uri https://myAdress.com/ ;

Possible Solution

Change in HtmlDumper.php those lines, which are injecting inline-css (inside html tag only) into the html output.

Delete the inline css that is placed inside the html-tags and place it inside a <style></style> section instead by assigning this css according to the targets classname or id.
The <style></style> section is also inline css but with teh difference that the <style> tag can easily be legitimated by adding a nonce with a str_replace() when outputting the dump().

Critical lines found that should be changed in HtmlDumper.php:

Line 160:
refStyle.innerHTML = 'pre.sf-dump .sf-dump-compact, .sf-dump-str-collapse .sf-dump-str-collapse, .sf-dump-str-expand .sf-dump-str-expand { display: none; }';

Link:
https://github.com/symfony/var-dumper/blob/92a342cd476603a3e8404bc802626df14da46e93/Dumper/HtmlDumper.php#L160

Line 354
refStyle.innerHTML = 'pre.sf-dump .'+a[0]+'{background-color: #B729D9; color: #FFF !important; border-radius: 2px}';

Link:
https://github.com/symfony/var-dumper/blob/92a342cd476603a3e8404bc802626df14da46e93/Dumper/HtmlDumper.php#L354

Additional Context

No response

@derrabus
Copy link
Member

I'm not sure I'd consider this a bug because making debug output CSP compliant wasn't really a requirement until now. However, if you know a way to fix this for you, feel free to create a PR so we can ship this as a new feature for 6.3.

@oioix
Copy link
Author

oioix commented Jan 23, 2023

Thanks for your quick reply.
CSP compatibility is certainly not a disadvantage. After all, the CSP technology is now developing more and more into a standard to ward off XSS.

I will have a look on it, as soon I find the time to.

@wouterj
Copy link
Member

wouterj commented Jan 23, 2023

Btw, for the sake of the security team in your company, I would recommend not sending mails or logging it in a centralized CSP-violation log if CSP violation occur in local environments on the developer's machines :)

@oioix
Copy link
Author

oioix commented Jan 23, 2023

Of course all incoming input of CSP report is filtered/sanitized. ,-)

@alexislefebvre
Copy link
Contributor

alexislefebvre commented Jan 25, 2023

If someone else wonders, CSP is Content Security Policy https://developer.mozilla.org/docs/Web/HTTP/CSP

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@oioix
Copy link
Author

oioix commented Aug 1, 2023

Hey, thanks for your report! There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

I just tested it.
Yes, this bug is still relevant.

No, I could not find a solution for this bug yet.

@javiereguiluz
Copy link
Member

As reported in #51692, in addition to style-src, we also need to solve the issue for the script-src and <style> elements.

@damienfa
Copy link
Contributor

I noticed the same issue with the {{ dump() }} display and the CSP.
The CSP-violation makes the "VarDump display" works bad (colapse arrow on array are not working anymore etc.).
The <script> has not "nonce" so my browser deny it and the expand/collapse in the sf-dump is not working for example.
Ex: <script>Sfdump("sf-dump-1795286044")</script>

It would be great to fix it and to be CSP compliant 😕

Thanks !

@wouterj
Copy link
Member

wouterj commented Jan 25, 2024

Is there anyone open to work on this? Seems like some community members are interested in this, so we need someone to experiment on how to solve this.

(PS, it's not needed to duplicate your comment on multiple issues, especially not on closed ones. Issues often don't need extra attention, they just need someone that thinks it's important enough to put effort into solving them)

@mustanggb
Copy link

mustanggb commented Jun 7, 2024

Here is the commit that added nonce support to the Toolbar (etc.): symfony/web-profiler-bundle@1050eec

Presumably we'd need to do something like that?

If so, should the relevant code be duplicated, used as is (VarDumper requires WebProfilerBundle?), or moved to a more common location?

It's kind of annoying having to disable CSP every time to do any debugging.

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

No branches or pull requests

8 participants