CVE-2023-36828: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Statamic is a flat-first, Laravel and Git powered content management system. Prior to version 4.10.0, the SVG tag does not sanitize malicious SVG. Therefore, an attacker can exploit this vulnerability to perform cross-site scripting attacks using SVG, even when using the `sanitize` function. Version 4.10.0 contains a patch for this issue.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2023-07-05T22:15Z
Repository: https://github.com/statamic/cms
Language: PHP
Files changed: composer.json, src/Tags/Svg.php, tests/Tags/SvgTagTest.php
Commit date: 2023-07-05 12:26:57 -0400
Commit message: [4.x] SVG tag sanitization (#8408)


Diff: diff --git a/composer.json b/composer.json
index 17ca9f01b..79ec0a290 100644
--- a/composer.json
+++ b/composer.json
@@ -10,10 +10,11 @@
     "license": "proprietary",
     "require": {
         "ext-json": "*",
         "ajthinking/archetype": "^1.0.3",
         "composer/composer": "^1.10.22 || ^2.2.12",
+        "enshrined/svg-sanitize": "^0.16.0",
         "facade/ignition-contracts": "^1.0",
         "guzzlehttp/guzzle": "^6.3 || ^7.0",
         "james-heinrich/getid3": "^1.9.21",
         "laravel/framework": "^9.50.0 || ^10.0",
         "laravel/helpers": "^1.1",
diff --git a/src/Tags/Svg.php b/src/Tags/Svg.php
index ec4ae3b38..8e55c374e 100644
--- a/src/Tags/Svg.php
+++ b/src/Tags/Svg.php
@@ -1,9 +1,12 @@
 <?php
 
 namespace Statamic\Tags;
 
+use enshrined\svgSanitize\data\AllowedAttributes;
+use enshrined\svgSanitize\data\AllowedTags;
+use enshrined\svgSanitize\Sanitizer;
 use Statamic\Facades\File;
 use Statamic\Facades\URL;
 use Statamic\Support\Str;
 use Stringy\StaticStringy;
 
@@ -45,15 +48,17 @@ class Svg extends Tags
 
         if ($this->params->get('title') || $this->params->get('desc')) {
             $svg = $this->setTitleAndDesc($svg);
         }
 
-        return str_replace(
+        $svg = str_replace(
             '<svg',
             collect(['<svg', $attributes])->filter()->implode(' '),
             $svg
         );
+
+        return $this->sanitize($svg);
     }
 
     private function setTitleAndDesc($svg)
     {
         $doc = new \DOMDocument;
@@ -77,6 +82,60 @@ class Svg extends Tags
             }
         }
 
         return $doc->saveHTML();
     }
+
+    private function sanitize($svg)
+    {
+        if ($this->params->bool('sanitize') === false) {
+            return $svg;
+        }
+
+        $sanitizer = new Sanitizer;
+        $sanitizer->removeXMLTag(! Str::startsWith($svg, '<?xml'));
+        $sanitizer->setAllowedAttrs($this->getAllowedAttrs());
+        $sanitizer->setAllowedTags($this->getAllowedTags());
+
+        return $sanitizer->sanitize($svg);
+    }
+
+    private function getAllowedAttrs()
+    {
+        $attrs = $this->params->explode('allow_attrs', []);
+
+        return new class($attrs) extends AllowedAttributes
+        {
+            private static $attrs = [];
+
+            public function __construct($attrs)
+            {
+                self::$attrs = $attrs;
+            }
+
+            public static function getAttributes()
+            {
+                return array_merge(parent::getAttributes(), self::$attrs);
+            }
+        };
+    }
+
+    private function getAllowedTags()
+    {
+        $tags = $this->params->explode('allow_tags', []);
+
+        return new class($tags) extends AllowedTags
+        {
+            private static $tags = [];
+
+            public function __construct($tags)
+            {
+                self::$tags = $tags;
+            }
+
+            public static function getTags()
+            {
+                return array_merge(parent::getTags(), self::$tags);
+            }
+        };
+    }
 }
diff --git a/tests/Tags/SvgTagTest.php b/tests/Tags/SvgTagTest.php
index aa24b9ad7..7dc98c98a 100644
--- a/tests/Tags/SvgTagTest.php
+++ b/tests/Tags/SvgTagTest.php
@@ -2,10 +2,11 @@
 
 namespace Tests\Tags;
 
 use Illuminate\Support\Facades\File;
 use Statamic\Facades\Parse;
+use Statamic\View\Antlers\Language\Utilities\StringUtilities;
 use Tests\TestCase;
 
 class SvgTagTest extends TestCase
 {
     public function setUp(): void
@@ -15,11 +16,14 @@ class SvgTagTest extends TestCase
         File::copy(__DIR__.'/../../resources/svg/icons/light/users.svg', resource_path('users.svg'));
     }
 
     private function tag($tag)
     {
-        return Parse::template($tag, []);
+        $output = Parse::template($tag, []);
+
+        // Normalize whitespace and line breaks for testing ease.
+        return trim(StringUtilities::normalizeLineEndings($output));
     }
 
     /** @test */
     public function it_renders_svg()
     {
@@ -30,6 +34,72 @@ class SvgTagTest extends TestCase
     /** @test */
     public function it_renders_svg_with_additional_params()
     {
         $this->assertStringStartsWith('<svg class="mb-2" xmlns="', $this->tag('{{ svg src="users" class="mb-2" }}'));
     }
+
+    /** @test */
+    public function it_sanitizes()
+    {
+        File::put(resource_path('xss.svg'), <<<'SVG'
+<svg>
+    <path onload="loadxss" onclick="clickxss"></path>
+    <script>alert("xss")</script>
+    <foreignObject></foreignObject>
+    <mesh></mesh>
+</svg>
+SVG);
+
+        $this->assertEquals(<<<'SVG'
+<svg>
+  <path></path>
+</svg>
+SVG,
+            $this->tag('{{ svg src="xss" sanitize="true" }}')
+        );
+
+        $this->assertEquals(<<<'SVG'
+<svg>
+  <path onclick="clickxss"></path>
+  <foreignObject></foreignObject>
+  <mesh></mesh>
+</svg>
+SVG,
+            $this->tag('{{ svg src="xss" sanitize="true" allow_tags="mesh|foreignObject" allow_attrs="onclick" }}')
+        );
+    }
+
+    /** @test */
+    public function sanitizing_doesnt_add_xml_tag()
+    {
+        // Thes sanitizer package adds an xml tag by default.
+        // We want to make sure if there wasn't one to begin with, it doesn't add one.
+
+        $svg = <<<'SVG'
+<svg>
+  <path></path>
+</svg>
+SVG;
+
+        File::put(resource_path('xmltag.svg'), $svg);
+
+        $this->assertEquals($svg, $this->tag('{{ svg src="xmltag" sanitize="true" }}'));
+    }
+
+    /** @test */
+    public function sanitizing_doesnt_remove_an_xml_tag()
+    {
+        // Thes sanitizer package adds an xml tag by default.
+        // We want to make sure that we haven't configured it to remove it if we wanted it there to begin with.
+
+        $svg = <<<'SVG'
+<?xml version="1.0" encoding="UTF-8"?>
+<svg>
+  <path></path>
+</svg>
+SVG;
+
+        File::put(resource_path('xmltag.svg'), $svg);
+
+        $this->assertEquals($svg, $this->tag('{{ svg src="xmltag" sanitize="true" }}'));
+    }
 }

Vulnerable code:         return str_replace(
        return Parse::template($tag, []);
Fixed code:         "enshrined/svg-sanitize": "^0.16.0",
use enshrined\svgSanitize\data\AllowedAttributes;
use enshrined\svgSanitize\data\AllowedTags;
use enshrined\svgSanitize\Sanitizer;
        $svg = str_replace(

        return $this->sanitize($svg);

    private function sanitize($svg)
    {
        if ($this->params->bool('sanitize') === false) {
            return $svg;
        }

        $sanitizer = new Sanitizer;
        $sanitizer->removeXMLTag(! Str::startsWith($svg, '<?xml'));
        $sanitizer->setAllowedAttrs($this->getAllowedAttrs());
        $sanitizer->setAllowedTags($this->getAllowedTags());

        return $sanitizer->sanitize($svg);
    }

    private function getAllowedAttrs()
    {
        $attrs = $this->params->explode('allow_attrs', []);

        return new class($attrs) extends AllowedAttributes
        {
            private static $attrs = [];

            public function __construct($attrs)
            {
                self::$attrs = $attrs;
            }

            public static function getAttributes()
            {
                return array_merge(parent::getAttributes(), self::$attrs);
            }
        };
    }

    private function getAllowedTags()
    {
        $tags = $this->params->explode('allow_tags', []);

        return new class($tags) extends AllowedTags
        {
            private static $tags = [];

            public function __construct($tags)
            {
                self::$tags = $tags;
            }

            public static function getTags()
            {
                return array_merge(parent::getTags(), self::$tags);
            }
        };
    }
use Statamic\View\Antlers\Language\Utilities\StringUtilities;
        $output = Parse::template($tag, []);

        // Normalize whitespace and line breaks for testing ease.
        return trim(StringUtilities::normalizeLineEndings($output));

    /** @test */
    public function it_sanitizes()
    {
        File::put(resource_path('xss.svg'), <<<'SVG'
<svg>
    <path onload="loadxss" onclick="clickxss"></path>
    <script>alert("xss")</script>
    <foreignObject></foreignObject>
    <mesh></mesh>
</svg>
SVG);

        $this->assertEquals(<<<'SVG'
<svg>
  <path></path>
</svg>
SVG,
            $this->tag('{{ svg src="xss" sanitize="true" }}')
        );

        $this->assertEquals(<<<'SVG'
<svg>
  <path onclick="clickxss"></path>
  <foreignObject></foreignObject>
  <mesh></mesh>
</svg>
SVG,
            $this->tag('{{ svg src="xss" sanitize="true" allow_tags="mesh|foreignObject" allow_attrs="onclick" }}')
        );
    }

    /** @test */
    public function sanitizing_doesnt_add_xml_tag()
    {
        // Thes sanitizer package adds an xml tag by default.
        // We want to make sure if there wasn't one to begin with, it doesn't add one.

        $svg = <<<'SVG'
<svg>
  <path></path>
</svg>
SVG;

        File::put(resource_path('xmltag.svg'), $svg);

        $this->assertEquals($svg, $this->tag('{{ svg src="xmltag" sanitize="true" }}'));
    }

    /** @test */
    public function sanitizing_doesnt_remove_an_xml_tag()
    {
        // Thes sanitizer package adds an xml tag by default.
        // We want to make sure that we haven't configured it to remove it if we wanted it there to begin with.

        $svg = <<<'SVG'
<?xml version="1.0" encoding="UTF-8"?>
<svg>
  <path></path>
</svg>
SVG;

        File::put(resource_path('xmltag.svg'), $svg);

        $this->assertEquals($svg, $this->tag('{{ svg src="xmltag" sanitize="true" }}'));
    }