New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user
Status: Fixed
Closed: Jan 2017
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

issue 680418

Sign in to add a comment
SVG animations on non-animatable elements.
Project Member Reported by, Jan 9 2017 Back to list
Clever folks have noted that it's possible to change the `href` attribute of a `<script>` element via things like `<set>` and `<animate>`.

This is somewhat useful for XSS attacks: injecting something like `<svg><set href=#x attributeName=href to=// />` or `<svg><animate href=#x attributeName=href to=// />` gets you code execution on the site.

The PoCs don't work in Firefox, and based on a quick skim of what I think is the relevant spec (, this shouldn't be possible. Am I reading that right? 

I've CC'd a few folks who know more about SVG than I do. Hopefully one of y'all can triage this to the right person. (fs@, I'm assigning it to you, since you're the last one to explicitly touch animation... thanks!)

I'm not marking this as RestrictView, as the discussion of the bug is already public.
Comment 1 by, Jan 9 2017
The list in the link there applies specifically to the <animateMotion> element, a more pertinent spec link might be (the "Animatable: no" bit mostly.) I think we end up falling into this hole because of how 'href' on SVGScriptElement is actually an SVGAnimatedString (because it implements SVGURIReference. SVG 1.1 had the same setup, so this is nothing new.) So we have an SVGAnimatedString that can never be animated! href.baseValilicious!
Comment 2 by, Jan 9 2017
Ok. Hrm. Can we subclass `SVGURIReference` into `SVGURIReferenceThatCanNotBeAnimatedBecauseWhoWouldEverAnimateHrefAttributesThatsWeird`? :)
Project Member Comment 3 by, Jan 9 2017
Status: Assigned
Comment 4 by, Jan 9 2017
I think an easier path of approach could be to remove this 'href' (and hence also 'xlink:href') from the property-map - that way the animation system would not see it as animatable.
Comment 5 by, Jan 9 2017
Can you walk me through doing that? I have zero idea how SVG works. :)
Comment 6 by, Jan 9 2017
Sure, just let me upload the CL first ;-)
Comment 8 by, Jan 11 2017
Status: Fixed
If we want to merge this (to 56 primarily), I don't think it'll merge cleanly, but rather needs some tending too. Hopefully not too complicated though.
Comment 9 by, Jan 12 2017
Blocking: 680418
Comment 10 by, Jan 12 2017
Labels: -Type-Bug-Security Type-Bug
Comment 11 by, Jan 12 2017
fs@: I don't think we _need_ to merge it to 56; it's not at all a new bug. If you have free time you'd like to fill, however, please go right ahead. :)
Comment 12 by, Jan 12 2017
Well, if I did think it'd merge cleanly I might've tried already, but since it likely won't, I'll spend those minutes somewhere else until otherwise prompted... =)
Sign in to add a comment