Skip to content

Commit bfd7470

Browse files
committed
fix($sanitize): don't allow svg animation tags
After angular#11124 got merged, a security vulnerability got introduced. Animation in SVG became tolerated by the sanitizer. Exploit Example: ``` <svg> <a xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="?"> <circle r="400"></circle> <animate attributeName="xlink:href" begin="0" from="javascript:alert(1)" to="&" /> </a> </svg> ``` Here we are animating an anchor's href, starting from a value that's a javascript URI, allowing the executing of arbitrary javascript in the process. Preventing only the animation of links is tricky, as SVG is weird and namespaces aren't predictable. We've decided to have the sanitizer filter out svg animation tags instead. Considering the sanitizer is commonly used to sanitize untrusted HTML code, this shouldn't affect many apps in the wild. Also, no release has been with angular#11124 in it, but not this fix.
1 parent a0957e9 commit bfd7470

File tree

2 files changed

+12
-4
lines changed

2 files changed

+12
-4
lines changed

src/ngSanitize/sanitize.js

+3-4
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,9 @@ var inlineElements = angular.extend({}, optionalEndTagInlineElements, makeMap("a
206206

207207
// SVG Elements
208208
// https://wiki.whatwg.org/wiki/Sanitization_rules#svg_Elements
209-
var svgElements = makeMap("animate,animateColor,animateMotion,animateTransform,circle,defs," +
210-
"desc,ellipse,font-face,font-face-name,font-face-src,g,glyph,hkern,image,linearGradient," +
211-
"line,marker,metadata,missing-glyph,mpath,path,polygon,polyline,radialGradient,rect,set," +
212-
"stop,svg,switch,text,title,tspan,use");
209+
var svgElements = makeMap("circle,defs,desc,ellipse,font-face,font-face-name,font-face-src,g,glyph," +
210+
"hkern,image,linearGradient,line,marker,metadata,missing-glyph,mpath,path,polygon,polyline," +
211+
"radialGradient,rect,set,stop,svg,switch,text,title,tspan,use");
213212

214213
// Special Elements (can contain anything)
215214
var specialElements = makeMap("script,style");

test/ngSanitize/sanitizeSpec.js

+9
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,15 @@ describe('HTML', function() {
273273
.toEqual('<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><a></a></svg>');
274274
});
275275

276+
it('should not accept SVG animation tags', function() {
277+
expectHTML('<svg xmlns:xlink="http://www.w3.org/1999/xlink"><a><text y="1em">Click me</text><animate attributeName="xlink:href" values="javascript:alert(1)"/></a></svg>')
278+
.toEqual('<svg xmlns:xlink="http://www.w3.org/1999/xlink"><a><text y="1em">Click me</text></a></svg>');
279+
280+
expectHTML('<svg><a xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="?"><circle r="400"></circle>' +
281+
'<animate attributeName="xlink:href" begin="0" from="javascript:alert(1)" to="&" /></a></svg>')
282+
.toEqual('<svg><a xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="?"><circle r="400"></circle></a></svg>');
283+
});
284+
276285
describe('htmlSanitizerWriter', function() {
277286
/* global htmlSanitizeWriter: false */
278287
if (angular.isUndefined(window.htmlSanitizeWriter)) return;

0 commit comments

Comments
 (0)