Closed Bug 1262842 Opened 8 years ago Closed 4 years ago

[CSP] Blocks the use of style attributes inside SVG without generating console errors

Categories

(Core :: DOM: Security, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla77
Webcompat Priority ?
Tracking Status
firefox64 --- wontfix
firefox65 --- wontfix
firefox77 --- fixed

People

(Reporter: April, Assigned: ckerschb)

References

(Blocks 2 open bugs, )

Details

(Whiteboard: [domsecurity-active])

Attachments

(6 files, 1 obsolete file)

On my website (https://pokeinthe.io), I had an issue where SVG icons were frequently rendering in pure black, instead of with color.  They are generated by my design program (Affinity Designer) and look like this:

> <?xml version="1.0" standalone="no"?>
> <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
> <svg width="100%" height="100%" viewBox="0 0 256 250" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xml:space="preserve" style="fill-rule:evenodd;clip-rule:evenodd;stroke-linejoin:round;stroke-miterlimit:1.41421;">
>     <path d="..." style="fill:rgb(246,188,246);fill-rule:nonzero;"/>
> </svg>

In particular, they have something like these style attributes:

> style="fill-rule:evenodd;clip-rule:evenodd;stroke-linejoin:round;stroke-miterlimit:1.41421;"
> style="fill:rgb(246,188,246);fill-rule:nonzero;"

It took me a long time to figure out what was happening.  It turns out it was due to my site-wide CSP policy:

> Content-Security-Policy:
>   default-src 'none';
>   child-src https://air.mozilla.org;
>   font-src 'self' https://fonts.gstatic.com;
>   frame-ancestors 'none';
>   frame-src https://air.mozilla.org;
>   img-src 'self';
>   media-src 'self';
>   script-src 'self';
>   style-src 'self' https://fonts.googleapis.com

It turns out that Firefox was treating these SVG files as having 'unsafe-inline' and thus blocking the loading of those attributes.  Unfortunately, as you can see in the screenshots, there is no indication that this is happening in the console (black-svg-due-to-csp-no-error-in-console.png). It's in fact *doubly* confusing, because it looks perfectly fine in the network tab (confusing-network-tab.png). To be honest, it's actually *triply* confusing, because Chromium browsers have no problem with these SVG files at all, and so there were no leads in their dev tools either.

The only way I could fix it was enabling a custom CSP header just for SVG files (colored-svg-due-to-custom-csp-policy-for-svg.png).

> default-src 'none'; frame-ancestors 'none'; style-src 'self' 'unsafe-inline'

Inside nginx:

> add_header Content-Security-Policy "default-src 'none'; child-src https://air.mozilla.org; font-src 'self' https://fonts.gstatic.com; frame-ancestors 'none'; frame-src https://air.mozilla.org; img-src 'self'; media-src 'self'; script-src 'self'; style-src 'self' https://fonts.googleapis.com";
> 
> location ~ \.svg$ {
>     add_header Content-Security-Policy "default-src 'none'; frame-ancestors 'none'; style-src 'self' 'unsafe-inline'";
> }

See: https://github.com/w3c/webappsec-csp/issues/45 for related discussion.
Whiteboard: [domsecurity-backlog]
Blocks: csp-w3c-3
Pasting/updating my description from bug 1312240:

We recently implemented CSP on gimp.org the most secure way, i.e. without 'unsafe-inline': https://observatory.mozilla.org/analyze.html?host=www.gimp.org

Actual results: We had a small Wilber icon in the top (just left to "GIMP" item in the top black bar) and it ended up black (hence barely visible) because the style is fully inline (as any SVG file straight out of Inkscape): https://www.gimp.org/
It was not spotted until recently (SVG images passed under the inline script/css cleaning radar). All other SVG images used on our website have the same issue.

Note that I found a workaround for the time being: since our styling needs are mostly basic, I could convert all inline CSS into XML attributes ("Optimized SVG" export in Inkscape) with minimal loss, then I clean out all remaining style with a regexp. But this is far from an optimal workflow to have to process every SVG image through an export then do additional cleaning.

https://bugzilla.gnome.org/show_bug.cgi?id=773364

Expected results:
I checked the CSP spec and according to section "3.6 Policy Applicability", when a SVG is embedded via <img>:

> No policy; should be just as safe as JPG

https://www.w3.org/TR/CSP11/#which-policy-applies
So if not mistaken, in our case, since the Wilber icon is indeed included as <img>, Firefox should not apply any policy on the subresource, hence it should apply the inline CSS in the SVG icon.

Also I can confirm that it works as requested by the spec in Chrome: the CSP policy is applied if I display the SVG standalone for instance (no styling, it becomes dark: https://www.gimp.org/images/wilbericon.svg), but it appears ok, with styling, when used as `<img>` (in gimp.org).
This is affecting portswigger.net too. We aren't comfortable with enabling unsafe-inline for styles as a workaround, as this opens up a lot of attack surface.
Priority: -- → P3
The language from comment 4 doesn't seem to be in the current CSP spec draft at https://w3c.github.io/webappsec-csp/ nor in https://www.w3.org/TR/CSP3/

That said, looks like CSP is only honored on documents in browsing contexts in general (per https://w3c.github.io/webappsec-csp/#initialize-document-csp which is only called from the HTML spec during navigation).

Christoph, is my understanding there correct?  If so, why are we applying CSP to SVG images loaded via <img> at all?
Flags: needinfo?(ckerschb)
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #11)
> Christoph, is my understanding there correct?  If so, why are we applying
> CSP to SVG images loaded via <img> at all?

Your understanding is absolutely correct. As far as I can tell this is simply a bug in our implementation and we should eventually fix it.
Flags: needinfo?(ckerschb)
WFM in current Nightly using
> Content-Security-Policy: default-src 'self'
and a SVG with inline CSS:
> Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”).
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
(In reply to sjw from comment #14)
> WFM in current Nightly using
> > Content-Security-Policy: default-src 'self'
> and a SVG with inline CSS:
> > Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”).

Did you only test that there was a warning when the inline style is blocked?

Did you also test that an SVG loaded as <svg> loaded fine (i.e. its style was not blocked) despite CSP, since there were kind of 2 issues in this report. Cf. comment 4, later confirmed as a bug in comment 11 and comment 12.
I just tested with Nightly (65.0a1, 2018-10-29), and not only does the bug not seem to be fixed but I also don't see warnings in the console.

:sjw, were you accessing the .svg files directly? That was never the problem. The problem was SVG files loaded as a subresource.
Status: RESOLVED → REOPENED
Flags: needinfo?(sjw)
Resolution: WORKSFORME → ---
> Did you only test that there was a warning when the inline style is blocked?

I only checked for the console warning. If the CSP shouldn't be applied to the loaded SVG (I used <img>), than I can confirm that the bug does still exist. Can we change the summary to clarify what the main issue is?
Flags: needinfo?(sjw)
As far as I can tell, neither one of the two issues raised by this bug are solved.

Which test site are you using to see if you are getting console errors? Do you know if there is a patch associated with fixing the console messaging?
I noticed some broken SVGs in a development environment after switching to a more restrictive CSP. There the warning I posted in the console and changing the inline styles to attributes fixed the issue for me.

I just created a simple test case (I think it's not possible to test this when uploaded to Bugzilla):

svg-csp.html:
> <!DOCTYPE html>
> <html>
>         <head>
>                 <meta charset="utf-8">
>                 <meta http-equiv="Content-Security-Policy" content="default-src 'self'">
>                  <title>Testcase</title>
>         </head>
>         <body>
>                 <img src="svg-csp.svg">
>         </body>
> </html>

svg-csp.svg:
> <svg width="150" height="150" viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
>         <style>
>                 circle {
>                         fill: orange;
>                         stroke: black;
>                         stroke-width: 10px;
>                 }
>         </style>
> 
>         <circle cx="50" cy="50" r="40" />
> </svg>

Here are some observations:
* When you open svg-csp.svg you can see the image as expected.
* When you open svg-csp.html you can see the image as expected.
* When you open svg-csp.html and refresh, the circle turns black and an error is logged:
> Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”). svg-csp.html:2:1

The last issue seems to be something new, related to local files. I didn't need to force refresh when the file was server from my development HTTP server.
I tested with a fresh profile in stable and nightly.

The SVG is a modified example from https://developer.mozilla.org/en-US/docs/Web/SVG/Element/style
Attached image svg-csp.svg
Attached file Testcase
I added the testcase anyway, but as expected the CSP can not be overwritten in Bugzilla.
I just undid my fix as noted in comment #1.

If you go to https://pokeinthe.io/ and click on the "Content" dropdown at the top, you will see that the top three SVG files are blacked out (due to CSP) and that no errors occur in the console.
Err, I meant "Contact", not "Content".
Yes, I can reproduce this. I only see a warning if I access https://pokeinthe.io/theme/images/icons/linkedin.svg directly.
I changed my testcase to load the image by CSS, but I still get the warning there.
Note that CSP specification states that it is not supposed to be applied to SVG files loaded via <img> tags.
I just experienced it while setting up csp for a site that i'm working on, is this something Mozilla is working on ? Otherwise how can we help ?
Are you taking this on, or can you help find an owner for this bug? We may also want to reconsider the priority (currently set to P3)
Flags: needinfo?(april)
I'm pretty sure that :ckerschb is the owner of this, but I do think that violating the CSP specification in this way should probably be a higher priority for fixing.

I get emails or tweets from people thanking me for writing my blog post about this problem at least once a week - it's the top Google result for "CSP SVG".
Flags: needinfo?(april)
This has also been changed in Edge recently:
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/7657500/
Christoph, is this something we may want to think about fixing for 65? It looks like we keep steadily getting duplicate reports, which I would take as a sign we should bump the priority on the issue. What do you think?
Flags: needinfo?(ckerschb)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #31)
> Christoph, is this something we may want to think about fixing for 65? It
> looks like we keep steadily getting duplicate reports, which I would take as
> a sign we should bump the priority on the issue. What do you think?

I tried to test this using the attached testcase. In the regular case I see the svg-csp.svg (black circle with orange in the middle). If I change the CSP to default-src 'none' I only see a black circle (no orange), but then I also get the following message in the browser console as well as the web console:

> Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”). svg-csp.html:2:1

Then I tried your testcase from Comment 8. When I go to https://pokeinthe.io/ and click on the 'Contact' menu I see blacked out SVGs in the menu.



Going forward, within Bug 965637 we are moving the CSP from the Principal into the Client (LoadInfo) which will fix a variety of problems around CSP (ultimately also allows us to apply CSP to system privileged pages, see Bug 1492063 and dependents). Anyway, when apply the patches from Bug 965637 both of the above mentioned cases seem to work. Bug 965637 is already in review and I expect it to land within this cycle. I will mark this bug depending on Bug 965637. If there are still issues after Bug 965637 has landed, we will follow up on it.
Depends on: 965637
Flags: needinfo?(ckerschb)

I've the same issue here on a website (and some other websites might encounter this issue, as this technique is already used), using a SVG sprite file from CSS:

sprite-for-css-only.svg

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="16" height="16" viewBox="0 0 16 16">

  <style> 
  use { display: none; } 
  use:target { display: block; } 
  
  .white { fill: #fff; } 
  .black { fill: #000; } 
  </style> 

  <symbol id="symbol-search"> 
  <path d="M9.6,8.89a4.47,4.47,0,0,0,1-2.83,4.56,4.56,0,1,0-4.56,4.56,4.47,4.47,0,0,0,2.83-1l4.26,4.25.7-.7Zm-3.54.73A3.56,3.56,0,1,1,9.62,6.06,3.57,3.57,0,0,1,6.06,9.62Z"></path>
  </symbol> 

  <symbol id="symbol-carret"> 
  <path d="M7.782 9.356l-4.44-4.111-.75.694 5.19 4.805 5.19-4.805-.749-.694z"/>
  </symbol> 
  
  <use id="css-search" xlink:href="#symbol-search" class="black" />
  <use id="css-carret" xlink:href="#symbol-carret" class="black" />

</svg>

and referring to it from CSS via: background: url("/assets/img/sprite-for-css-only.svg#css-search") 6px 50% no-repeat;
It should display only the use specified, which is a quite smart and useful.

My CSP headers are set up not to allow unsafe-inline CSS, and the inlne styles of the SVG files are blocked (which goes to display all SVG paths) :(
I've found a stupid workaround by adding a hash in style-src directive (a nonce works also, but as it is in a static SVG file, not a good idea).

Please fix this, this is a really annoying bug.

I have a case of the website branding losing its color. This is a very significant issue, it is not acceptable that svg's lose their color, webdevs are forced to lose up CSP as long as this bug exists.

Case: https://ffscoiety.nl

That website is still in development and what we see is a placeholder, but as you can see an important choice is coming up, it can't go into production like that. The SVG should have a distinct blue gradient.

Point I am trying to make: This is indeed a high priority issue, it forces developers to lose up their security. Kindly suggest bumping priority higher than P3.

Note that if you disable CSP on SVG images, it will display properly while still giving you full security. See the first post for how.

You proposed workaround can be quite dangerous, as setting a header within the location block means that all other headers set previously no longer apply to svg files:

From nginx http://nginx.org/en/docs/http/ngx_http_headers_module.html :

"There could be several add_header directives. These directives are inherited from the previous level if and only if there are no add_header directives defined on the current level. "

If I am not mistaken a location directive adds a new level.
Doing it your way means I devs must be aware of this and set all their headers again on location ~ .svg$

This is hitting another site, as reported at https://webcompat.com/issues/41955

The issue affects SVG images even when viewing them directly in the URL bar: https://portswigger.net/web-security/images/browser-logos/firefox.svg

I would prefer that we address this sooner rather than later, as it is not immediately obvious to someone diagnosing a webcompat issue what is happening due to the lack of console feedback (unless they already know about this bug).

Webcompat Priority: --- → ?

Christoph, now that bug 965637 is fixed, what's left to do to fix this?

Flags: needinfo?(ckerschb)

(In reply to Boris Zbarsky [:bzbarsky] from comment #41)

Christoph, now that bug 965637 is fixed, what's left to do to fix this?

There is nothing left to fix this. Seems bug 965637 took care of it. Uploading a test in a minute.

Assignee: nobody → ckerschb
Status: REOPENED → ASSIGNED
Flags: needinfo?(ckerschb)
Priority: P3 → P2
Whiteboard: [domsecurity-backlog] → [domsecurity-active]
Attachment #9137440 - Attachment description: Bug 1262842: Test inline script within svg image. r=april → Bug 1262842: Test inline style within svg image. r=april

The initial bug report here involved a site-wide CSP delivered via HTTP headers, including for SVG images, no? So that test is not testing the initial bug report's setup...

Flags: needinfo?(ckerschb)

(In reply to Boris Zbarsky [:bzbarsky] from comment #44)

The initial bug report here involved a site-wide CSP delivered via HTTP headers, including for SVG images, no? So that test is not testing the initial bug report's setup...

I agree, the test is using a meta CSP and does not rely on an http header delivered CSP. FWIW, I flagged April (the reporter) for review on the test and also pinged her on slack - but I am happy to update the test if needed.

Flags: needinfo?(ckerschb)

I just tested on my website, and Nightly doesn't seem to be working.

If there is a CSP delivered on SVG file loaded in the context of a webpage, it still blocks the use of inline styles inside them.

(In reply to April King [:April] from comment #46)

I just tested on my website, and Nightly doesn't seem to be working.
If there is a CSP delivered on SVG file loaded in the context of a webpage, it still blocks the use of inline styles inside them.

Ah, so my testcase is wrong in the end. The SVG itself is served using a CSP not allowing inline style. I thought the SVG incorrecty inherits the CSP from the loading document. So there is still a bug. Since I am already on it, I will rewrite my automated testcase and fix that. Thanks Boris and April!

Attachment #9137440 - Attachment is obsolete: true
Blocks: 1627235
No longer blocks: 1627235
Depends on: 1627235
Pushed by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a70c352cc910
CSP served on SVG images should be ignored, otherwise SVG image internals like inline styles might get blocked. r=smaug
Status: ASSIGNED → RESOLVED
Closed: 6 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
See Also: → 1810563
See Also: → 1459872
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: