From a6b39334c69226c2ab3b6184d95c89e2207c441a Mon Sep 17 00:00:00 2001
From: Adam Holdbrook <adam.holdbrook@technologywithin.com>
Date: Wed, 7 Aug 2024 12:00:56 +0100
Subject: [PATCH] Make sure nested php tags are also removed when sanitising
 svg

---
 src/Sanitizer.php                     |  9 +++-
 tests/SanitizerTest.php               | 74 +++++++++++++++++++--------
 tests/data/maliciousJsAndPhpClean.svg | 11 ++++
 tests/data/maliciousJsAndPhpTest.svg  | 13 +++++
 4 files changed, 85 insertions(+), 22 deletions(-)
 create mode 100644 tests/data/maliciousJsAndPhpClean.svg
 create mode 100644 tests/data/maliciousJsAndPhpTest.svg

diff --git a/src/Sanitizer.php b/src/Sanitizer.php
index 9fcab00..c0cb120 100644
--- a/src/Sanitizer.php
+++ b/src/Sanitizer.php
@@ -220,8 +220,13 @@ public function sanitize($dirty)
             return '';
         }
 
-        // Strip php tags
-        $dirty = preg_replace('/<\?(=|php)(.+?)\?>/i', '', $dirty);
+        do {
+            /*
+             * recursively remove php tags because they can be hidden inside tags
+             * i.e. <?p<?php test?>hp echo . ' danger! ';?>
+             */
+            $dirty = preg_replace('/<\?(=|php)(.+?)\?>/i', '', $dirty);
+        } while (preg_match('/<\?(=|php)(.+?)\?>/i', $dirty) != 0);
 
         $this->resetInternal();
         $this->setUpBefore();
diff --git a/tests/SanitizerTest.php b/tests/SanitizerTest.php
index 97514f0..11cdf96 100644
--- a/tests/SanitizerTest.php
+++ b/tests/SanitizerTest.php
@@ -308,34 +308,34 @@ public function testLargeUseDOSattacksAreNullified()
         self::assertXmlStringEqualsXmlString($expected, $cleanData);
     }
 
-	public function testInvalidNodesAreHandled()
-	{
-		$dataDirectory = __DIR__ . '/data';
-		$initialData = file_get_contents($dataDirectory . '/htmlTest.svg');
-		$expected = file_get_contents($dataDirectory . '/htmlClean.svg');
+    public function testInvalidNodesAreHandled()
+    {
+        $dataDirectory = __DIR__ . '/data';
+        $initialData = file_get_contents($dataDirectory . '/htmlTest.svg');
+        $expected = file_get_contents($dataDirectory . '/htmlClean.svg');
 
-		$sanitizer = new Sanitizer();
-		$sanitizer->minify(false);
-		$cleanData = $sanitizer->sanitize($initialData);
+        $sanitizer = new Sanitizer();
+        $sanitizer->minify(false);
+        $cleanData = $sanitizer->sanitize($initialData);
 
-		self::assertXmlStringEqualsXmlString($expected, $cleanData);
-	}
+        self::assertXmlStringEqualsXmlString($expected, $cleanData);
+    }
 
     /**
      * @test
      */
-	public function cdataSectionIsSanitized()
-	{
-		$dataDirectory = __DIR__ . '/data';
-		$initialData = file_get_contents($dataDirectory . '/cdataTest.svg');
-		$expected = file_get_contents($dataDirectory . '/cdataClean.svg');
+    public function cdataSectionIsSanitized()
+    {
+        $dataDirectory = __DIR__ . '/data';
+        $initialData = file_get_contents($dataDirectory . '/cdataTest.svg');
+        $expected = file_get_contents($dataDirectory . '/cdataClean.svg');
 
-		$sanitizer = new Sanitizer();
-		$sanitizer->minify(false);
-		$cleanData = $sanitizer->sanitize($initialData);
+        $sanitizer = new Sanitizer();
+        $sanitizer->minify(false);
+        $cleanData = $sanitizer->sanitize($initialData);
 
-		self::assertXmlStringEqualsXmlString($expected, $cleanData);
-	}
+        self::assertXmlStringEqualsXmlString($expected, $cleanData);
+    }
 
     /**
      * @test
@@ -368,4 +368,38 @@ public function formDataisSanitized()
 
         self::assertXmlStringEqualsXmlString($expected, $cleanData);
     }
+
+    /**
+     * @test
+     */
+    public function maliciousSvgJsSanitized()
+    {
+        $dataDirectory = __DIR__ . '/data';
+        $initialData = file_get_contents($dataDirectory . '/maliciousJsAndPhpTest.svg');
+        $expected = file_get_contents($dataDirectory . '/maliciousJsAndPhpClean.svg');
+
+
+        $sanitizer = new Sanitizer();
+        $sanitizer->minify(false);
+        $cleanData = $sanitizer->sanitize($initialData);
+
+        self::assertXmlStringEqualsXmlString($expected, $cleanData);
+    }
+
+    /**
+     * @test
+     */
+    public function maliciousSvgPhpTagsStripped()
+    {
+        $dataDirectory = __DIR__ . '/data';
+        $initialData = file_get_contents($dataDirectory . '/maliciousJsAndPhpTest.svg');
+
+        $sanitizer = new Sanitizer();
+        $sanitizer->minify(false);
+        $cleanData = $sanitizer->sanitize($initialData);
+
+        foreach (['<?php', '<?='] as $value) {
+            self::assertStringNotContainsString($value, $cleanData);
+        }
+    }
 }
diff --git a/tests/data/maliciousJsAndPhpClean.svg b/tests/data/maliciousJsAndPhpClean.svg
new file mode 100644
index 0000000..411cea5
--- /dev/null
+++ b/tests/data/maliciousJsAndPhpClean.svg
@@ -0,0 +1,11 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <image/>
+    <svg/>
+
+    <defs/>
+    <g>
+        <circle/>
+        <text/>
+    </g>
+    <!--  -->
+</svg>
diff --git a/tests/data/maliciousJsAndPhpTest.svg b/tests/data/maliciousJsAndPhpTest.svg
new file mode 100644
index 0000000..1913859
--- /dev/null
+++ b/tests/data/maliciousJsAndPhpTest.svg
@@ -0,0 +1,13 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <test></test>
+    <image onload="alert(1)"></image>
+    <svg onload="alert(2)"></svg>
+    <script>alert(3)</script>
+    <defs onload="alert(4)"></defs>
+    <g onload="alert(5)">
+        <circle onload="alert(6)" />
+        <text onload="alert(7)"></text>
+    </g>
+    <ø:script src="//0x.lv/" />
+    <!-- <?p<?php test?>hp echo ' Hello Danger! ';?> -->
+</svg>