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

xlink:href="data:," attribute on crown image is triggering Content Security Policy error #2203

Closed
paulwaitehomeoffice opened this issue Apr 23, 2021 · 3 comments · Fixed by #2229
Assignees
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) header ⚠️ high priority Used by the team when triaging 🔍 investigation
Milestone

Comments

@paulwaitehomeoffice
Copy link

paulwaitehomeoffice commented Apr 23, 2021

Description of the issue

Previous releases of govuk-frontend included the crown logo using this HTML:

<image src="/assets/images/govuk-logotype-crown.png" xlink:href="" class="govuk-header__logotype-crown-fallback-image" width="36" height="32"></image>

A recent update (I'm not sure which, but I'm getting the issue in 3.11.0, having upgraded from 3.9.1) has inserted the content data:, in the xlink:href attribute:

<image src="/assets/images/govuk-logotype-crown.png" xlink:href="data:," display="none" class="govuk-header__logotype-crown-fallback-image" width="36" height="32"></image>

Our Content Security Policy for does not allow data: URL images, and it seems like this attribute is being interpreted (by Chrome and Safari at least) as an attempt to load a data: URL image.

I'm not sure if this is just the browsers being over-zealous, but I'm also not clear why the xlink:href attribute is needed.

Edit: I forgot I could just look at the source to see if there are comments about the Xlink:href attribute, and indeed there are:

The <image> element is a valid SVG element. In SVG, you would specify

So I think it's there to prevent SVG-supporting versions of Internet Explorer from unnecessarily downloading the fallback PNG image?

Steps to reproduce the issue

  1. Create a site using govuk-frontend 3.11.0
  2. Implement a Content Security Policy on the site that does not allow data: URL images (for example, img-src 'self' www.google-analytics.com)
  3. Load the site in a browser that supports Content Security Policy

Actual vs expected behaviour

Expected: no console errors.
Actual:

Refused to load the image 'data:,' because it violates the following Content Security Policy directive: "img-src 'self' www.google-analytics.com".

Environment (where applicable)

  • Operating system: macOS Catalina 10.15.7
  • Browser: Chrome
  • Browser version: 90.0.4430.85 (Official Build) (x86_64)
  • GOV.UK Frontend Version: 3.11.0
@paulwaitehomeoffice paulwaitehomeoffice added awaiting triage Needs triaging by team 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) labels Apr 23, 2021
@andymantell
Copy link
Contributor

This is where the xlink:href came in:

#2045

@36degrees 36degrees added header ⚠️ high priority Used by the team when triaging 🔍 investigation 🕔 days and removed awaiting triage Needs triaging by team labels Apr 26, 2021
@36degrees 36degrees added this to the [NEXT] milestone Apr 30, 2021
tombye added a commit to alphagov/emergency-alerts-govuk that referenced this issue May 6, 2021
The current fallback method works like this:
- we put a `<image>` tag in the SVG that uses an
  empty data URI for the image data and the SVG
  attribute of `display="none"` so it doesn't
  show up in browsers that support SVG
- the `<image>` tag includes a `src` attribute,
  pointing to a fallback image

Because browsers interpret `<image>` as `<img>`,
those that don't support SVG:
- ignore all the SVG tags
- interpret `<image>` as an `<img>` tag and
  display the image its `src` points to

The problem with this is that using data URIs
violates our Content Security Policy.

The new fallback method is to just wrap SVGs in
conditional comments that ensure it is only
delivered to Internet Explorer (IE) 8 and above, as well as all
other browsers. This does mean that any browsers
that don't support SVG but aren't IE will no
longer get the fallback but this seems limited to
quite old versions of Android's native browser:

https://caniuse.com/svg

The latest one of these was released in 2010 so I
think this risk is reasonable.

These changes extend the interface of the
alerts_icon component to allow the conditional
comments to be optional. This is because we nest
it in the example component, which is wrapped in
conditional comments itself so any child SVGs it
includes don't need them.

Note: The very latest GOVUK Frontend also contains
this issue but it has been raised with the team
and is scheduled to be worked on soon:

alphagov/govuk-frontend#2203

We don't use this version yet so can hold off
until they roll out a solution (which we may yet
copy).
tombye added a commit to alphagov/emergency-alerts-govuk that referenced this issue May 6, 2021
The current fallback method works like this:
- we put a `<image>` tag in the SVG that uses an
  empty data URI for the image data and the SVG
  attribute of `display="none"` so it doesn't
  show up in browsers that support SVG
- the `<image>` tag includes a `src` attribute,
  pointing to a fallback image

Because browsers interpret `<image>` as `<img>`,
those that don't support SVG:
- ignore all the SVG tags
- interpret `<image>` as an `<img>` tag and
  display the image its `src` points to

The problem with this is that using data URIs
violates our Content Security Policy.

The new fallback method is to just wrap SVGs in
conditional comments that ensure it is only
delivered to Internet Explorer (IE) 8 and above, as well as all
other browsers. This does mean that any browsers
that don't support SVG but aren't IE will no
longer get the fallback but this seems limited to
quite old versions of Android's native browser:

https://caniuse.com/svg

The latest one of these was released in 2010 so I
think this risk is reasonable.

These changes extend the interface of the
alerts_icon component to allow the conditional
comments to be optional. This is because we nest
it in the example component, which is wrapped in
conditional comments itself so any child SVGs it
includes don't need them.

Note: The very latest GOVUK Frontend also contains
this issue but it has been raised with the team
and is scheduled to be worked on soon:

alphagov/govuk-frontend#2203

We don't use this version yet so can hold off
until they roll out a solution (which we may yet
copy).
tombye added a commit to alphagov/emergency-alerts-govuk that referenced this issue May 6, 2021
The current fallback method works like this:
- we put a `<image>` tag in the SVG that uses an
  empty data URI for the image data and the SVG
  attribute of `display="none"` so it doesn't
  show up in browsers that support SVG
- the `<image>` tag includes a `src` attribute,
  pointing to a fallback image

Because browsers interpret `<image>` as `<img>`,
those that don't support SVG:
- ignore all the SVG tags
- interpret `<image>` as an `<img>` tag and
  display the image its `src` points to

The problem with this is that using data URIs
violates our Content Security Policy.

The new fallback method is to just wrap SVGs in
conditional comments that ensure it is only
delivered to Internet Explorer (IE) 8 and above, as well as all
other browsers. This does mean that any browsers
that don't support SVG but aren't IE will no
longer get the fallback but this seems limited to
quite old versions of Android's native browser:

https://caniuse.com/svg

The latest one of these was released in 2010 so I
think this risk is reasonable.

These changes extend the interface of the
alerts_icon component to allow the conditional
comments to be optional. This is because we nest
it in the example component, which is wrapped in
conditional comments itself so any child SVGs it
includes don't need them.

Note: The very latest GOVUK Frontend also contains
this issue but it has been raised with the team
and is scheduled to be worked on soon:

alphagov/govuk-frontend#2203

We don't use this version yet so can hold off
until they roll out a solution (which we may yet
copy).
@36degrees
Copy link
Contributor

Looking into this, one option may be to switch to using conditional comments to switch between the SVG and a standard HTML <img> tag:

<!--[if gt IE 8]><!-->
<svg aria-hidden="true" focusable="false" class="govuk-header__logotype-crown" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 132 97" height="30" width="36">
  <path fill="currentColor" fill-rule="evenodd" d="M25 30.2c3.5 1.5 7.7-.2 9.1-3.7 1.5-3.6-.2-7.8-3.9-9.2-3.6-1.4-7.6.3-9.1 3.9-1.4 3.5.3 7.5 3.9 9zM9 39.5c3.6 1.5 7.8-.2 9.2-3.7 1.5-3.6-.2-7.8-3.9-9.1-3.6-1.5-7.6.2-9.1 3.8-1.4 3.5.3 7.5 3.8 9zM4.4 57.2c3.5 1.5 7.7-.2 9.1-3.8 1.5-3.6-.2-7.7-3.9-9.1-3.5-1.5-7.6.3-9.1 3.8-1.4 3.5.3 7.6 3.9 9.1zm38.3-21.4c3.5 1.5 7.7-.2 9.1-3.8 1.5-3.6-.2-7.7-3.9-9.1-3.6-1.5-7.6.3-9.1 3.8-1.3 3.6.4 7.7 3.9 9.1zm64.4-5.6c-3.6 1.5-7.8-.2-9.1-3.7-1.5-3.6.2-7.8 3.8-9.2 3.6-1.4 7.7.3 9.2 3.9 1.3 3.5-.4 7.5-3.9 9zm15.9 9.3c-3.6 1.5-7.7-.2-9.1-3.7-1.5-3.6.2-7.8 3.7-9.1 3.6-1.5 7.7.2 9.2 3.8 1.5 3.5-.3 7.5-3.8 9zm4.7 17.7c-3.6 1.5-7.8-.2-9.2-3.8-1.5-3.6.2-7.7 3.9-9.1 3.6-1.5 7.7.3 9.2 3.8 1.3 3.5-.4 7.6-3.9 9.1zM89.3 35.8c-3.6 1.5-7.8-.2-9.2-3.8-1.4-3.6.2-7.7 3.9-9.1 3.6-1.5 7.7.3 9.2 3.8 1.4 3.6-.3 7.7-3.9 9.1zM69.7 17.7l8.9 4.7V9.3l-8.9 2.8c-.2-.3-.5-.6-.9-.9L72.4 0H59.6l3.5 11.2c-.3.3-.6.5-.9.9l-8.8-2.8v13.1l8.8-4.7c.3.3.6.7.9.9l-5 15.4v.1c-.2.8-.4 1.6-.4 2.4 0 4.1 3.1 7.5 7 8.1h.2c.3 0 .7.1 1 .1.4 0 .7 0 1-.1h.2c4-.6 7.1-4.1 7.1-8.1 0-.8-.1-1.7-.4-2.4V34l-5.1-15.4c.4-.2.7-.6 1-.9zM66 92.8c16.9 0 32.8 1.1 47.1 3.2 4-16.9 8.9-26.7 14-33.5l-9.6-3.4c1 4.9 1.1 7.2 0 10.2-1.5-1.4-3-4.3-4.2-8.7L108.6 76c2.8-2 5-3.2 7.5-3.3-4.4 9.4-10 11.9-13.6 11.2-4.3-.8-6.3-4.6-5.6-7.9 1-4.7 5.7-5.9 8-.5 4.3-8.7-3-11.4-7.6-8.8 7.1-7.2 7.9-13.5 2.1-21.1-8 6.1-8.1 12.3-4.5 20.8-4.7-5.4-12.1-2.5-9.5 6.2 3.4-5.2 7.9-2 7.2 3.1-.6 4.3-6.4 7.8-13.5 7.2-10.3-.9-10.9-8-11.2-13.8 2.5-.5 7.1 1.8 11 7.3L80.2 60c-4.1 4.4-8 5.3-12.3 5.4 1.4-4.4 8-11.6 8-11.6H55.5s6.4 7.2 7.9 11.6c-4.2-.1-8-1-12.3-5.4l1.4 16.4c3.9-5.5 8.5-7.7 10.9-7.3-.3 5.8-.9 12.8-11.1 13.8-7.2.6-12.9-2.9-13.5-7.2-.7-5 3.8-8.3 7.1-3.1 2.7-8.7-4.6-11.6-9.4-6.2 3.7-8.5 3.6-14.7-4.6-20.8-5.8 7.6-5 13.9 2.2 21.1-4.7-2.6-11.9.1-7.7 8.8 2.3-5.5 7.1-4.2 8.1.5.7 3.3-1.3 7.1-5.7 7.9-3.5.7-9-1.8-13.5-11.2 2.5.1 4.7 1.3 7.5 3.3l-4.7-15.4c-1.2 4.4-2.7 7.2-4.3 8.7-1.1-3-.9-5.3 0-10.2l-9.5 3.4c5 6.9 9.9 16.7 14 33.5 14.8-2.1 30.8-3.2 47.7-3.2z"></path>
</svg>
<!--<![endif]-->
<!--[if IE 8]>
<img src="/assets/images/govuk-logotype-crown.png" class="govuk-header__logotype-crown-fallback-image" width="36" height="32">
<![endif]-->

The downside of this approach is that it only works for users of Internet Explorer 8. Users of other older browsers that do not support SVG (for example Android Browser 2.1-2.3) would not see the fallback image.

We're trying to get some additional data to help understand what percentage of users this might affect, including:

  • traffic to GOV.UK from Android Browser 2.1-2.3
  • requests for the fallback PNG to the CDN, broken down by user agent

Given that the last release of Android Browser 2.3 was in 2010, and Android 4.3 and below does not support TLS 1.2, it seems unlikely we'll be seeing much traffic from these devices.

It's also curious that it's not included in the list of possible approaches in https://css-tricks.com/svg-fallbacks/, which makes me wonder if there's another downside we're not yet aware of.

@36degrees
Copy link
Contributor

Visits to GOV.UK from Android 2.2 devices compared to Internet Explorer 8, between 2020-05-14 and 2021-04-30, according to Google Analytics:

Users Percentage of all users Sessions Perecentage of all sessions
All users 366,139,809 - 1,664,725,083 -
Android 77,144,107 21.1% 577,929,096 34.7%
Android 2.2 4,343 0.00119% 6,792 0.00041%
Internet Explorer 8 28,963 0.00791% 36,075 0.00217%

Still trying to get hold of CDN data to see if there are other user agents we should be considering.

36degrees added a commit that referenced this issue May 17, 2021
The current fallback uses an `<image>` element, which is interpreted differently depending on whether it’s parsed in the context of SVG or HTML.

We include an `xlink:href` attribute on the `<image>` tag to prevent versions of IE which support SVG from downloading the fallback image when they don't need to.

However, this approach has proved problematic – IE11 still requests the fallback PNG when printing, which can cause issues with sessions (#2045) and an attempted fix for that issue by using a blank data uri has now caused further issues with Content Security Policies (#2203).

Switch to the simpler approach of using conditional comments for the header fallback PNG, targeting Internet Explorer 8 specifically. We can then use a good old-fashioned `<img>` tag.

This does mean that browsers that do not support SVG other than Internet Explorer 8 will not be able to see the crown emblem.

According to Can I Use, the other main browser that does not support SVG that we may need to consider is Android 2.1 - 2.3 [1].

Between 2020-05-14 and 2021-04-30 analytics for GOV.UK shows 6,792 (0.00041%) sessions from 4,343 (0.00119%) users using Android 2.x, out of 577,929,096 sessions from 77,144,107 users.

By comparison, in the same period we saw 36,075 sessions (0.00217%) from 28,963 users (0.00791%) using IE8.

It appears that Android 2.x browsers just omit the SVG entirely – there’s no ‘broken image’ displayed – these users just see ‘GOV.UK’ without the crown.

Given the very low volume of traffic from these browsers, this seems like an acceptable thing to do.

We could have removed the fallback PNG entirely, however our browser support explicitly states support for Internet Explorer 8 (‘although components may not look perfect’) and we know that some service teams do have a disproprtionate number of IE8 users – which is unlikely to be the case for Android 2.

[1]:https://caniuse.com/svg
36degrees added a commit that referenced this issue May 17, 2021
The current fallback uses an `<image>` element, which is interpreted differently depending on whether it’s parsed in the context of SVG or HTML.

We include an `xlink:href` attribute on the `<image>` tag to prevent versions of IE which support SVG from downloading the fallback image when they don't need to.

However, this approach has proved problematic – IE11 still requests the fallback PNG when printing, which can cause issues with sessions (#2045) and an attempted fix for that issue by using a blank data uri has now caused further issues with Content Security Policies (#2203).

Switch to the simpler approach of using conditional comments for the header fallback PNG, targeting Internet Explorer 8 specifically. We can then use a good old-fashioned `<img>` tag.

This does mean that browsers that do not support SVG other than Internet Explorer 8 will not be able to see the crown emblem.

According to Can I Use, the other main browser that does not support SVG that we may need to consider is Android 2.1 - 2.3 [1].

Between 2020-05-14 and 2021-04-30 analytics for GOV.UK shows 6,792 (0.00041%) sessions from 4,343 (0.00119%) users using Android 2.x, out of 577,929,096 sessions from 77,144,107 users.

By comparison, in the same period we saw 36,075 sessions (0.00217%) from 28,963 users (0.00791%) using IE8.

It appears that Android 2.x browsers just omit the SVG entirely – there’s no ‘broken image’ displayed – these users just see ‘GOV.UK’ without the crown.

Given the very low volume of traffic from these browsers, this seems like an acceptable thing to do.

We could have removed the fallback PNG entirely, however our browser support explicitly states support for Internet Explorer 8 (‘although components may not look perfect’) and we know that some service teams do have a disproprtionate number of IE8 users – which is unlikely to be the case for Android 2.

[1]:https://caniuse.com/svg
36degrees added a commit that referenced this issue May 17, 2021
The current fallback uses an `<image>` element, which is interpreted differently depending on whether it’s parsed in the context of SVG or HTML.

We include an `xlink:href` attribute on the `<image>` tag to prevent versions of IE which support SVG from downloading the fallback image when they don't need to.

However, this approach has proved problematic – IE11 still requests the fallback PNG when printing, which can cause issues with sessions (#2045) and an attempted fix for that issue by using a blank data uri has now caused further issues with Content Security Policies (#2203).

Switch to the simpler approach of using conditional comments for the header fallback PNG, targeting Internet Explorer 8 specifically. We can then use a good old-fashioned `<img>` tag.

This does mean that browsers that do not support SVG other than Internet Explorer 8 will not be able to see the crown emblem.

According to Can I Use, the other main browser that does not support SVG that we may need to consider is Android 2.1 - 2.3 [1].

Between 2020-05-14 and 2021-04-30 analytics for GOV.UK shows 6,792 (0.00041%) sessions from 4,343 (0.00119%) users using Android 2.x, out of 577,929,096 sessions from 77,144,107 users.

By comparison, in the same period we saw 36,075 sessions (0.00217%) from 28,963 users (0.00791%) using IE8.

It appears that Android 2.x browsers just omit the SVG entirely – there’s no ‘broken image’ displayed – these users just see ‘GOV.UK’ without the crown.

Given the very low volume of traffic from these browsers, this seems like an acceptable thing to do.

We could have removed the fallback PNG entirely, however our browser support explicitly states support for Internet Explorer 8 (‘although components may not look perfect’) and we know that some service teams do have a disproprtionate number of IE8 users – which is unlikely to be the case for Android 2.

[1]:https://caniuse.com/svg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) header ⚠️ high priority Used by the team when triaging 🔍 investigation
Projects
4 participants