From 0c6617af440879ce97440f6eb6c58636456dc8ec Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Wed, 9 Oct 2019 15:36:32 -0400
Subject: [PATCH] mitigate XSS vulnerability in SVG animate attributes

this addresses CVE-2019-15587

see #171 for more information

https://github.com/flavorjones/loofah/issues/171
---
 lib/loofah/html5/safelist.rb    |  3 ---
 test/integration/test_ad_hoc.rb | 30 ++++++++++++++++++++++++------
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/lib/loofah/html5/safelist.rb b/lib/loofah/html5/safelist.rb
index 8abd922..4b2b6dd 100644
--- a/lib/loofah/html5/whitelist.rb
+++ b/lib/loofah/html5/whitelist.rb
@@ -88,7 +88,7 @@
 
       SVG_ATTRIBUTES = Set.new %w[accent-height accumulate additive alphabetic
        arabic-form ascent attributeName attributeType baseProfile bbox begin
-       by calcMode cap-height class clip-path clip-rule color
+       calcMode cap-height class clip-path clip-rule color
        color-interpolation-filters color-rendering content cx cy d dx
        dy descent display dur end fill fill-opacity fill-rule
        filterRes filterUnits font-family
@@ -105,9 +105,9 @@
        stemv stop-color stop-opacity strikethrough-position
        strikethrough-thickness stroke stroke-dasharray stroke-dashoffset
        stroke-linecap stroke-linejoin stroke-miterlimit stroke-opacity
-       stroke-width systemLanguage target text-anchor to transform type u1
+       stroke-width systemLanguage target text-anchor transform type u1
        u2 underline-position underline-thickness unicode unicode-range
-       units-per-em values version viewBox visibility width widths x
+       units-per-em version viewBox visibility width widths x
        x-height x1 x2 xlink:actuate xlink:arcrole xlink:href xlink:role
        xlink:show xlink:title xlink:type xml:base xml:lang xml:space xmlns
        xmlns:xlink y y1 y2 zoomAndPan]
diff --git a/test/integration/test_ad_hoc.rb b/test/integration/test_ad_hoc.rb
index 16fccbb..cc6fc65 100644
--- a/test/integration/test_ad_hoc.rb
+++ b/test/integration/test_ad_hoc.rb
@@ -190,14 +190,32 @@ def test_dont_remove_whitespace_between_tags
       end
     end
 
-    # see:
-    # - https://github.com/flavorjones/loofah/issues/154
-    # - https://hackerone.com/reports/429267
-    context "xss protection from svg xmlns:xlink animate attribute" do
-      it "sanitizes appropriate attributes" do
-        html = %Q{<svg><a xmlns:xlink=http://www.w3.org/1999/xlink xlink:href=?><circle r=400 /><animate attributeName=xlink:href begin=0 from=javascript:alert(1) to=%26>}
+    context "xss protection from svg animate attributes" do
+      # see recommendation from https://html5sec.org/#137
+      # to sanitize "to", "from", "values", and "by" attributes
+
+      it "sanitizes 'from', 'to', and 'by' attributes" do
+        # for CVE-2018-16468
+        # see:
+        # - https://github.com/flavorjones/loofah/issues/154
+        # - https://hackerone.com/reports/429267
+        html = %Q{<svg><a xmlns:xlink=http://www.w3.org/1999/xlink xlink:href=?><circle r=400 /><animate attributeName=xlink:href begin=0 from=javascript:alert(1) to=%26 by=5>}
+
         sanitized = Loofah.scrub_fragment(html, :escape)
         assert_nil sanitized.at_css("animate")["from"]
+        assert_nil sanitized.at_css("animate")["to"]
+        assert_nil sanitized.at_css("animate")["by"]
+      end
+
+      it "sanitizes 'values' attribute" do
+        # for CVE-2019-15587
+        # see:
+        # - https://github.com/flavorjones/loofah/issues/171
+        # - https://hackerone.com/reports/709009
+        html = %Q{<svg> <animate href="#foo" attributeName="href" values="javascript:alert('xss')"/> <a id="foo"> <circle r=400 /> </a> </svg>}
+
+        sanitized = Loofah.scrub_fragment(html, :escape)
+        assert_nil sanitized.at_css("animate")["values"]
       end
     end
   end