fix($sanitize): blacklist SVG <use> elements

[petebacondarwin]

The use element can reference external svg's (same origin) and can include
xlink javascript urls or foreign object that can execute xss.

This change disallows <use> elements in sanitized SVG markup.

An example of a malicious SVG document would be:

SVG to sanitize:

<svg><use xlink:href="test.svg#xss" /></svg>

External SVG file (test.svg)

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg xmlns:svg="http://www.w3.org/2000/svg"
   xmlns="http://www.w3.org/2000/svg" width="100"
   height="100"
   id="xss">
<a xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="javascript:alert(1)">
  <circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red" />
</a>
</svg>

Here the SVG to sanitize loads in the test.svg file via the <use> element.
The sanitizer is not able to parse this file, which contains malicious
executable mark-up.

This can only be taken advantage of if the external file is available via the
same origin restrictions in place.

[lgalfaso]

LGTM

On Saturday, December 5, 2015, Pete Bacon Darwin [email protected]
wrote:

The use element can reference external svg's (same origin) and can include
xlink javascript urls or foreign object that can execute xss.

This change disallows elements in sanitized SVG markup.

An example of a malicious SVG document would be:

SVG to sanitize:

External SVG file (test.svg)

Here the SVG to sanitize loads in the test.svg file via the element.
The sanitizer is not able to parse this file, which contains malicious
executable mark-up.

This can only be taken advantage of if the external file is available via
the

same origin restrictions in place.

You can view, comment on, or merge this pull request online at:

#13453
Commit Summary

• fix($sanitize): blacklist SVG <use> elements

File Changes

• M src/ngSanitize/sanitize.js
  https://github.com/angular/angular.js/pull/13453/files#diff-0 (2)
• M test/ngSanitize/sanitizeSpec.js
  https://github.com/angular/angular.js/pull/13453/files#diff-1 (7)

Patch Links:

• https://github.com/angular/angular.js/pull/13453.patch
• https://github.com/angular/angular.js/pull/13453.diff

—
Reply to this email directly or view it on GitHub
#13453.

[petebacondarwin]

Easy one :-) Does it need a breaking change notice?

[IgorMinar]

Add breaking change note please. Otherwise lgtm

[Coridyn]

We use the <use> element to reference SVG content defined on the page (e.g. as an alternative to Glyphicons):

<a href="#"><svg class="icon"><use xlink:href="#icon-call"></use></svg> Give us a call</a>

Would it be possible to opt in to <use> elements, allow local-only references (like above), or share the $compileProvider.imgSrcSanitizationWhitelist when sanitizing this sort of content?

Alternatively, are you aware of any workaround we could use without having to use trustAsHtml()?

[petebacondarwin]

@Coridyn perhaps you could come up with a custom directive so you are not placing svg directly in the bound HTML content?

[Coridyn]

@petebacondarwin The SVG is coming from user-editable content so I don't think that's possible.

Does running the content through $compile has the same security implications as using trustAsHtml()?