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

SVG Symbols violate strict CSP style-src directives #16827

Open
4 of 7 tasks
mikelkew opened this issue Jun 24, 2020 · 8 comments
Open
4 of 7 tasks

SVG Symbols violate strict CSP style-src directives #16827

mikelkew opened this issue Jun 24, 2020 · 8 comments

Comments

@mikelkew
Copy link

Describe the bug
When using SVG symbols (using data-fa-symbol) with the NPM package, inline styling is added here to hide the SVG symbol. However, with a strict Content Security Policy that doesn't include unsafe-inline for the style-src directive, the inline styles are rejected. This results in a CSP violation being reposted, and the SVG symbols not being correctly hidden.

To Reproduce
Set a CSP header of Content-Security-Policy: default-src 'self'; style-src 'self' and then try to use the SVG symbol feature.

Expected behavior
I suppose the best approach would be to add a css class to the symbol instead of the inline style here and add that class in styles.css. For those that haven't disabled config.autoAddCss, this should still work fine, and for those that have, at least it will provide a class that can be styled manually in the page CSS.

Screenshots
Example of rejected inline style and CSP violation:
Screenshot 2020-06-24 21 54 24

Version and implementation
Version: @fortawesome/fontawesome-svg-core@^1.2.28

  • SVG with JS
  • Web Fonts with CSS
  • SVG Sprites
  • On the Desktop

Bug report checklist

  • I have filled out as much of the above information as I can
  • I have included a test case because my odds go way up that the team can fix this when I do
  • I have searched for existing issues and to the best of my knowledge this is not a duplicate
@tagliala
Copy link
Member

Hi!

Thanks for being part of the Font Awesome Community and thanks for this detailed report.

@robmadole @mlwilkerson could you please take a look?

@mikelkew
Copy link
Author

Thanks @tagliala, created a quick example PR to demonstrate a proposed solution: #16832

@robmadole
Copy link
Member

@mikelkew thanks for the report. I don't think we can change it as you've suggested. That would break backward compatibility.

It would be possible (though ornery) to use the AST generated, modify it, and then output the results.

https://fontawesome.com/how-to-use/javascript-api/methods/icon#advanced

@mikelkew
Copy link
Author

Thanks @robmadole, that's understandable. I guess one way to preserve backward compatibility would be to add a configuration option for it. I've just updated the example PR at #16832 to use that approach. I can also completely understand if you'd rather not go down that path though, especially as it looks like inline styles are used in a few other functions as well.

In the meantime, for our purposes we've resorted to just including the SVG manually rather than relying on the JS to render it. It would be nice to be able to do it "natively", but we can get by without it if we have to!

@TheHokieCoder
Copy link

@robmadole What would be the break in backward compatibility with switching from inline to class-based styling? Perhaps if we understood what is tied directly to the inline styling, we could help come up with a workaround that wouldn't break the compatibility.

The workaround that I have used in my applications is to create a container DIV where I use a CSS class to set the display property to none. E.g.:

CSS

.fa-icon-container {
    display: none;
}

HTML

<div class="fa-icon-container">
    <i data-fa-symbol="external-link" class="far fa-external-link"></i>
    <i data-fa-symbol="right-arrow" class="fas fa-fw fa-arrow-circle-right"></i>
</div>

It works, but I still get the CSP errors in the browser console, and it is not ideal to have to add my own code/styling to fix something with an external library.

@MarcelloTheArcane
Copy link

I'm hitting CSP errors due to the use of insertCss functions like all of these.

DOCUMENT.head.insertBefore(style, beforeChild);

node.insertBefore(element, node.firstChild);

@tagliala
Copy link
Member

tagliala commented Apr 9, 2021

@robmadole I think we can consider this breaking change for the upcoming FA6

@bradleyhodges
Copy link

bradleyhodges commented Dec 9, 2022

Any update on this? We're on v6.2.1 and this is still an issue:

image

image

It doesn't seem like any meaningful progress has been made on this, and this oversight materially deminishes the ability for website operators to implement safe, effective content policies. As far as I can tell, the only way to bypass this is to use the unsafe-inline directive, which annuls most of the security benefits that Content-Security-Policy provides in the first place.

The alternative, of course, is to self-host, but why accept the tradeoff of not being able to use uploaded icons and any other benefits of Kits, especially considering that if we want to make use of those benefits, we have to open additional security holes to make it work.

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

6 participants