Modify URL normalization to better match the JS library

thephpleague · Mar 24, 2019 · 92ab7cb · 92ab7cb
1 parent 7606543
commit 92ab7cb
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,10 @@ Updates should follow the [Keep a CHANGELOG](http://keepachangelog.com/) princip
 
 ## [Unreleased][unreleased]
 
+### Changed
+
+ - Modified how URL normalization decodes certain characters in order to align with the JS library's output
+
 ## [0.18.3] - 2019-03-21
 
 This is a **security update** release.

diff --git a/src/Util/UrlEncoder.php b/src/Util/UrlEncoder.php
@@ -39,6 +39,20 @@ final class UrlEncoder
         '%7E' => '~',
     ];
 
+    protected static $dontDecode = [
+        ';',
+        '/',
+        '?',
+        ':',
+        '@',
+        '&',
+        '=',
+        '+',
+        '$',
+        ',',
+        '#',
+    ];
+
     /**
      * @param string $uri
      *
@@ -61,15 +75,13 @@ public static function unescapeAndEncode($uri)
     private static function decode($uri)
     {
         return preg_replace_callback('/%([0-9a-f]{2})/iu', function ($matches) {
-            // Convert percent-encoded codes to uppercase
-            $upper = strtoupper($matches[0]);
-            // Keep excluded characters as-is
-            if (array_key_exists($upper, self::$dontEncode)) {
-                return $upper;
+            $char = chr(hexdec($matches[1]));
+
+            if (in_array($char, self::$dontDecode, true)) {
+                return strtoupper($matches[0]);
             }
 
-            // Otherwise, return the character for this codepoint
-            return chr(hexdec($matches[1]));
+            return $char;
         }, $uri);
     }
 

diff --git a/tests/functional/data/safe_links/input.md b/tests/functional/data/safe_links/input.md
@@ -164,4 +164,4 @@ javas\x09cript:javascript:alert(1)>
 
 [XSS](javascript&colon;alert%28&#039;XSS&#039;%29)
 
-[XSS](javascript&amp;colon;alert%28&#039;XSS&#039;%29)
+[XSS](javascript&amp;colon;alert%28&#039;XSS&#039;%29)
diff --git a/tests/functional/data/safe_links/safe_output.html b/tests/functional/data/safe_links/safe_output.html
@@ -81,4 +81,4 @@
 <p><a>jAvAsCrIpT:alert(1)</a></p>
 <p><a>XSS</a></p>
 <p><a>XSS</a></p>
-<p><a href="javascript&amp;colon;alert%28'XSS'%29">XSS</a></p>
+<p><a href="javascript&amp;colon;alert('XSS')">XSS</a></p>
diff --git a/tests/functional/data/safe_links/unsafe_output.html b/tests/functional/data/safe_links/unsafe_output.html
@@ -79,6 +79,6 @@
 <p>&lt;jav&amp;#65ascript:javascript:alert(1)&gt;</p>
 <p>&lt;jav&amp;#97ascript:javascript:alert(1)&gt;</p>
 <p><a href="jAvAsCrIpT:alert(1)">jAvAsCrIpT:alert(1)</a></p>
-<p><a href="javascript:alert%28'XSS'%29">XSS</a></p>
-<p><a href="javascript:alert%28'XSS'%29">XSS</a></p>
-<p><a href="javascript&amp;colon;alert%28'XSS'%29">XSS</a></p>
+<p><a href="javascript:alert('XSS')">XSS</a></p>
+<p><a href="javascript:alert('XSS')">XSS</a></p>
+<p><a href="javascript&amp;colon;alert('XSS')">XSS</a></p>
diff --git a/tests/unit/Util/UrlEncoderTest.php b/tests/unit/Util/UrlEncoderTest.php
@@ -52,9 +52,29 @@ public function unescapeAndEncodeTestProvider()
             ['<', '%3C'],
             ['>', '%3E'],
             ['?', '?'],
+            ['%21', '!'],
+            ['%23', '%23'],
+            ['%24', '%24'],
+            ['%26', '%26'],
+            ['%27', "'"],
+            ['%2A', '*'],
+            ['%2B', '%2B'],
+            ['%2C', '%2C'],
+            ['%2D', '-'],
+            ['%2E', '.'],
+            ['%2F', '%2F'],
+            ['%3A', '%3A'],
+            ['%3B', '%3B'],
+            ['%3D', '%3D'],
+            ['%3F', '%3F'],
+            ['%40', '%40'],
+            ['%5F', '_'],
+            ['%7E', '~'],
             ['java%0ascript:alert("XSS")', 'java%0Ascript:alert(%22XSS%22)'],
             ['java%0Ascript:alert("XSS")', 'java%0Ascript:alert(%22XSS%22)'],
             ["java\nscript:alert('XSS')", "java%0Ascript:alert('XSS')"],
+            // Note that the following test does decode '&amp;colon;' to '&colon', but this does not impact rendering, which re-encodes that '&' elsewhere
+            ['javascript&amp;colon;alert%28&#039;XSS&#039;%29', 'javascript&colon;alert(&#039;XSS&#039;)'],
             ['https://en.wikipedia.org/wiki/Markdown#CommonMark', 'https://en.wikipedia.org/wiki/Markdown#CommonMark'],
             ['https://img.shields.io/badge/help-%23hoaproject-ff0066.svg', 'https://img.shields.io/badge/help-%23hoaproject-ff0066.svg'],
             ['http://example.com/a%62%63%2fd%3Fe', 'http://example.com/abc%2Fd%3Fe'],