-
Notifications
You must be signed in to change notification settings - Fork 12.2k
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
Comments
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 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 |
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! |
@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 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. |
I'm hitting CSP errors due to the use of Font-Awesome/js/fontawesome.js Line 643 in d79d85c
Font-Awesome/js/fontawesome.js Line 2051 in d79d85c
|
@robmadole I think we can consider this breaking change for the upcoming FA6 |
Any update on this? We're on v6.2.1 and this is still an issue: 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 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. |
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 includeunsafe-inline
for thestyle-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:
Version and implementation
Version:
@fortawesome/fontawesome-svg-core@^1.2.28
Bug report checklist
The text was updated successfully, but these errors were encountered: