Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 67688d5

Browse files
rodyhaddadpetebacondarwin
authored andcommitted
fix($sanitize): disallow unsafe svg animation tags
After #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 #11124 in it, but not this fix. Closes #11290
1 parent 9e8a687 commit 67688d5

File tree

2 files changed

+28
-19
lines changed

2 files changed

+28
-19
lines changed

src/ngSanitize/sanitize.js

+19-19
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,11 @@ 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+
// Note: the elements animate,animateColor,animateMotion,animateTransform,set are intentionally omitted.
210+
// They can potentially allow for arbitrary javascript to be executed. See #11290
211+
var svgElements = makeMap("circle,defs,desc,ellipse,font-face,font-face-name,font-face-src,g,glyph," +
212+
"hkern,image,linearGradient,line,marker,metadata,missing-glyph,mpath,path,polygon,polyline," +
213+
"radialGradient,rect,stop,svg,switch,text,title,tspan,use");
213214

214215
// Special Elements (can contain anything)
215216
var specialElements = makeMap("script,style");
@@ -233,21 +234,20 @@ var htmlAttrs = makeMap('abbr,align,alt,axis,bgcolor,border,cellpadding,cellspac
233234
// SVG attributes (without "id" and "name" attributes)
234235
// https://wiki.whatwg.org/wiki/Sanitization_rules#svg_Attributes
235236
var svgAttrs = makeMap('accent-height,accumulate,additive,alphabetic,arabic-form,ascent,' +
236-
'attributeName,attributeType,baseProfile,bbox,begin,by,calcMode,cap-height,class,color,' +
237-
'color-rendering,content,cx,cy,d,dx,dy,descent,display,dur,end,fill,fill-rule,font-family,' +
238-
'font-size,font-stretch,font-style,font-variant,font-weight,from,fx,fy,g1,g2,glyph-name,' +
239-
'gradientUnits,hanging,height,horiz-adv-x,horiz-origin-x,ideographic,k,keyPoints,' +
240-
'keySplines,keyTimes,lang,marker-end,marker-mid,marker-start,markerHeight,markerUnits,' +
241-
'markerWidth,mathematical,max,min,offset,opacity,orient,origin,overline-position,' +
242-
'overline-thickness,panose-1,path,pathLength,points,preserveAspectRatio,r,refX,refY,' +
243-
'repeatCount,repeatDur,requiredExtensions,requiredFeatures,restart,rotate,rx,ry,slope,stemh,' +
244-
'stemv,stop-color,stop-opacity,strikethrough-position,strikethrough-thickness,stroke,' +
245-
'stroke-dasharray,stroke-dashoffset,stroke-linecap,stroke-linejoin,stroke-miterlimit,' +
246-
'stroke-opacity,stroke-width,systemLanguage,target,text-anchor,to,transform,type,u1,u2,' +
247-
'underline-position,underline-thickness,unicode,unicode-range,units-per-em,values,version,' +
248-
'viewBox,visibility,width,widths,x,x-height,x1,x2,xlink:actuate,xlink:arcrole,xlink:role,' +
249-
'xlink:show,xlink:title,xlink:type,xml:base,xml:lang,xml:space,xmlns,xmlns:xlink,y,y1,y2,' +
250-
'zoomAndPan', true);
237+
'baseProfile,bbox,begin,by,calcMode,cap-height,class,color,color-rendering,content,' +
238+
'cx,cy,d,dx,dy,descent,display,dur,end,fill,fill-rule,font-family,font-size,font-stretch,' +
239+
'font-style,font-variant,font-weight,from,fx,fy,g1,g2,glyph-name,gradientUnits,hanging,' +
240+
'height,horiz-adv-x,horiz-origin-x,ideographic,k,keyPoints,keySplines,keyTimes,lang,' +
241+
'marker-end,marker-mid,marker-start,markerHeight,markerUnits,markerWidth,mathematical,' +
242+
'max,min,offset,opacity,orient,origin,overline-position,overline-thickness,panose-1,' +
243+
'path,pathLength,points,preserveAspectRatio,r,refX,refY,repeatCount,repeatDur,' +
244+
'requiredExtensions,requiredFeatures,restart,rotate,rx,ry,slope,stemh,stemv,stop-color,' +
245+
'stop-opacity,strikethrough-position,strikethrough-thickness,stroke,stroke-dasharray,' +
246+
'stroke-dashoffset,stroke-linecap,stroke-linejoin,stroke-miterlimit,stroke-opacity,' +
247+
'stroke-width,systemLanguage,target,text-anchor,to,transform,type,u1,u2,underline-position,' +
248+
'underline-thickness,unicode,unicode-range,units-per-em,values,version,viewBox,visibility,' +
249+
'width,widths,x,x-height,x1,x2,xlink:actuate,xlink:arcrole,xlink:role,xlink:show,xlink:title,' +
250+
'xlink:type,xml:base,xml:lang,xml:space,xmlns,xmlns:xlink,y,y1,y2,zoomAndPan', true);
251251

252252
var validAttrs = angular.extend({},
253253
uriAttrs,

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)