[!]: don't edit the encoding, if no XSS was detected

-> this will also fix issue #32
voku · Apr 15, 2018 · 49b1b8e · 49b1b8e
1 parent b6caf09
commit 49b1b8e
Show file tree

Hide file tree

Showing 7 changed files with 117 additions and 134 deletions.
diff --git a/src/voku/helper/AntiXSS.php b/src/voku/helper/AntiXSS.php
@@ -1787,13 +1787,10 @@ final class AntiXSS
       'onWebKitTransitionEnd',
       'onWheel',
       'seekSegmentTime',
-      'userid',
-      'datasrc',
-      'datafld',
-      'dataformatas',
       'ev:handler',
       'ev:event',
-      '0;url',
+      '&lt;script&gt;',
+      '&lt;/script&gt;',
   );
 
   /**
@@ -1855,13 +1852,6 @@ final class AntiXSS
       'xss',
   );
 
-  /**
-   * XSS Hash - random Hash for protecting URLs.
-   *
-   * @var  string
-   */
-  private $_xss_hash;
-
   /**
    * The replacement-string for not allowed strings.
    *
@@ -1911,7 +1901,7 @@ public function __construct()
    */
   private function _compact_exploded_words_callback($matches)
   {
-    return preg_replace('/(?:\s+|"|\042|\'|\047|\+)*+/', '', $matches[1]) . $matches[2];
+    return \preg_replace('/(?:\s+|"|\042|\'|\047|\+)*+/', '', $matches[1]) . $matches[2];
   }
 
   /**
@@ -1924,26 +1914,26 @@ private function _compact_exploded_words_callback($matches)
   private function _decode_entity($match)
   {
     // init
-    $this->_xss_hash();
-
-    $match = $match[0];
-
-    // protect GET variables in URLs
-    $match = preg_replace('|\?([a-z\_0-9\-]+)\=([a-z\_0-9\-/]+)|i', $this->_xss_hash . '::GET_FIRST' . '\\1=\\2', $match);
-    $match = preg_replace('|\&([a-z\_0-9\-]+)\=([a-z\_0-9\-/]+)|i', $this->_xss_hash . '::GET_NEXT' . '\\1=\\2', $match);
+    $str = $match[0];
+
+    // protect GET variables without XSS in URLs
+    if (\preg_match_all("/[\?|&]?[A-Za-z0-9_\-\[\]]+\s*=\s*(\"|\042|'|\047)([^\\1]*?)\\1/", $str, $matches)) {
+      if (isset($matches[2])) {
+        foreach ($matches[2] as $matchInner) {
+          $tmpAntiXss = clone $this;
+
+          $urlPartDecoded = $this->_entity_decode($matchInner);
+          $tmpAntiXss->xss_clean($urlPartDecoded);
+          if ($tmpAntiXss->isXssFound() === true) {
+            $str = \str_replace($matchInner, $urlPartDecoded, $str);
+          }
+        }
+      }
+    } else {
+      $str = $this->_entity_decode(UTF8::rawurldecode($str));
+    }
 
-    // un-protect URL GET vars
-    return str_replace(
-        array(
-            $this->_xss_hash . '::GET_FIRST',
-            $this->_xss_hash . '::GET_NEXT',
-        ),
-        array(
-            '?',
-            '&',
-        ),
-        $this->_entity_decode($match)
-    );
+    return $str;
   }
 
   /**
@@ -2066,7 +2056,7 @@ private function _do_never_allowed_afterwards($str)
 
     if (null === $NEVER_ALLOWED_STR_AFTERWARDS_CACHE) {
       foreach (self::$_never_allowed_str_afterwards as &$neverAllowedStr) {
-        $neverAllowedStr .= '.*=';
+        $neverAllowedStr .= '.*(?:=|%3D)';
       }
 
       $NEVER_ALLOWED_STR_AFTERWARDS_CACHE = implode('|', self::$_never_allowed_str_afterwards);
@@ -2096,11 +2086,7 @@ private function _entity_decode($str)
         ENT_QUOTES;
 
     // decode
-    if (strpos($str, $this->_xss_hash) !== false) {
-      $str = UTF8::html_entity_decode($str, $flags);
-    } else {
-      $str = UTF8::rawurldecode($str);
-    }
+    $str = UTF8::html_entity_decode($str, $flags);
 
     // decode-again, for e.g. HHVM, PHP 5.3, miss configured applications ...
     if (preg_match_all('/&[A-Za-z]{2,}[;]{0}/', $str, $matches)) {
@@ -2206,15 +2192,7 @@ private function _filter_attributes($str)
     }
 
     $out = '';
-    if (
-        preg_match_all('#\s*[A-Za-z\-]+\s*=\s*("|\042|\'|\047)([^\\1]*?)\\1#', $str, $matches)
-        ||
-        (
-            $this->_replacement
-            &&
-            preg_match_all('#\s*[a-zA-Z\-]+\s*=' . preg_quote($this->_replacement, '#') . '$#', $str, $matches)
-        )
-    ) {
+    if (preg_match_all('#\s*[A-Za-z0-9_\-\[\]]+\s*=\s*("|\042|\'|\047)([^\\1]*?)\\1#', $str, $matches)) {
       foreach ($matches[0] as $match) {
         $out .= $match;
       }
@@ -2263,7 +2241,7 @@ private function _initNeverAllowedStr()
    */
   private function _js_link_removal_callback($match)
   {
-    return $this->_js_removal_calback($match, 'href');
+    return $this->_js_removal_callback($match, 'href');
   }
 
   /**
@@ -2281,7 +2259,7 @@ private function _js_link_removal_callback($match)
    *
    * @return string
    */
-  private function _js_removal_calback($match, $search)
+  private function _js_removal_callback($match, $search)
   {
     if (!$match[0]) {
       return '';
@@ -2292,7 +2270,7 @@ private function _js_removal_calback($match, $search)
       \preg_match("/style=\".*?\"/i", $match[0], $match_style);
       $match_style_matched = (\count($match_style) > 0);
       if ($match_style_matched) {
-        $match[0] = \str_replace($match_style[0], $this->_xss_hash . '::STYLE', $match[0]);
+        $match[0] = \str_replace($match_style[0], 'voku::anti-xss::STYLE', $match[0]);
       }
     }
 
@@ -2315,7 +2293,7 @@ private function _js_removal_calback($match, $search)
     // hack for style attributes v2
     if ($search === 'href') {
       if ($match_style_matched) {
-        $return = \str_replace($this->_xss_hash . '::STYLE', $match_style[0], $return);
+        $return = \str_replace('voku::anti-xss::STYLE', $match_style[0], $return);
       }
     }
 
@@ -2338,7 +2316,7 @@ private function _js_removal_calback($match, $search)
    */
   private function _js_src_removal_callback($match)
   {
-    return $this->_js_removal_calback($match, 'src');
+    return $this->_js_removal_callback($match, 'src');
   }
 
   /**
@@ -2419,6 +2397,7 @@ private function _compact_exploded_javascript($str)
     $words = array(
         'javascript',
         'expression',
+        'ｅｘｐｒｅｓｓｉｏｎ',
         'view-source',
         'vbscript',
         'jscript',
@@ -2891,35 +2870,20 @@ public function xss_clean($str)
       return $str;
     }
 
+    $old_str_backup = $str;
+
     // process
     do {
       $old_str = $str;
       $str = $this->_do($str);
     } while ($old_str !== $str);
 
-    return $str;
-  }
-
-  /**
-   * Generates the XSS hash if needed and returns it.
-   *
-   * @return string <p>XSS hash</p>
-   */
-  private function _xss_hash()
-  {
-    if ($this->_xss_hash === null) {
-      $rand = Bootup::get_random_bytes(16);
-
-      if (!$rand) {
-        $this->_xss_hash = md5(uniqid(mt_rand(), true));
-      } else {
-        $this->_xss_hash = bin2hex($rand);
-      }
-
-      $this->_xss_hash = 'voku::anti-xss::' . $this->_xss_hash;
+    // keep the old encoding, if there wasn't any XSS attack
+    if ($this->xss_found !== true) {
+      $str = $old_str_backup;
     }
 
-    return $this->_xss_hash;
+    return (string)$str;
   }
 
 }
diff --git a/tests/JsXssTest.php b/tests/JsXssTest.php
@@ -37,7 +37,7 @@ public function testFromJsXss()
     self::assertSame('{a: 1111}', $this->security->xss_clean('{a: 1111}'));
 
     // 清除不可见字符
-    self::assertSame("a\r\n b", $this->security->xss_clean("a\u0000\u0001\u0002\u0003\r\n b"));
+    self::assertSame("a\u0000\u0001\u0002\u0003\r\n b", $this->security->xss_clean("a\u0000\u0001\u0002\u0003\r\n b"));
 
     // 过滤不在白名单的标签
     self::assertSame('<b>abcd</b>', $this->security->xss_clean('<b>abcd</b>'));
@@ -191,7 +191,7 @@ public function testFromJsXss()
     // HTML5新增实体编码 冒号&colon; 换行&NewLine;
     self::assertSame('<a href="">', $this->security->xss_clean('<a href="javascript&colon;alert(/xss/)">'));
     self::assertSame('<a href="">', $this->security->xss_clean('<a href="javascript&colonalert(/xss/)">'));
-    self::assertSame("<a href=\"a\nb\">", $this->security->xss_clean('<a href="a&NewLine;b">'));
+    self::assertSame("<a href=\"a&NewLine;b\">", $this->security->xss_clean('<a href="a&NewLine;b">'));
     self::assertSame('<a href="a&NewLineb">', $this->security->xss_clean('<a href="a&NewLineb">'));
     self::assertSame('<a href="">', $this->security->xss_clean('<a href="javasc&NewLine;ript&colon;alert(1)">'));
 

diff --git a/tests/LaravelSecurityTest.php b/tests/LaravelSecurityTest.php
@@ -91,7 +91,7 @@ public function snippetProvider()
         ),
         array(
             '<iframe/src="data:text/html,<iframe%09onload=confirm(1);>">',
-            '&lt;iframe/src="data:text/html,&lt;iframe	[removed]>">',
+            '&lt;iframe/src="data:text/html,&lt;iframe	[removed]confirm&#40;1&#41;;>">',
         ),
         array(
             '<math><a/xlink:href=javascript:prompt(1)>X',
@@ -147,7 +147,7 @@ public function snippetProvider()
         ),
         array(
             '<meta/http-equiv="refresh"/content="0;url=javascript&Tab;:&Tab;void(alert(0))?0:0,0,prompt(0)">',
-            '&lt;meta/http-equiv="refresh"/content="[removed][removed]	void(alert&#40;0&#41;)?0:0,0,prompt&#40;0&#41;"&gt;',
+            '&lt;meta/http-equiv="refresh"/content="0;url=[removed]	void(alert&#40;0&#41;)?0:0,0,prompt&#40;0&#41;"&gt;',
         ),
         array(
             '<script src="h&Tab;t&Tab;t&Tab;p&Tab;s&colon;/&Tab;/&Tab;http://dl.dropbox.com/u/13018058/js.js"></script>',

diff --git a/tests/XssTest.php b/tests/XssTest.php
diff --git a/tests/fixtures/expect.json b/tests/fixtures/expect.json
@@ -137,7 +137,7 @@
   {
     "title": "mXSS Variation I",
     "payload": "<listing>&lt;img onerror=\"alert(1);//\" src=1&gt;<t t></listing>",
-    "expected": "<listing><img ><></>"
+    "expected": "<listing><img \"><></>"
   },
   {
     "title": "mXSS Variation II",
@@ -365,11 +365,11 @@
   },
   {
     "payload": "<div id=\"29\"><link rel=stylesheet href=data:,*%7bx:expression(alert(29))%7d//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"29\">&lt;link rel=stylesheet href=data:,*{x:alert&#40;29&#41;)}//[\"'`--&gt;]]&gt;]</div>"
+    "expected": "<div id=\"29\">&lt;link rel=stylesheet href=data:,*%7bx:alert&#40;29&#41;)%7d//[\"'`--&gt;]]&gt;]</div>"
   },
   {
     "payload": "<div id=\"30\"><style>@import \"data:,*%7bx:expression(alert(30))%7D\";</style>//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"30\">&lt;style&gt;@import \"data:,*{x:alert&#40;30&#41;)}\";&lt;/style&gt;//[\"'`--&gt;]]>]</div>"
+    "expected": "<div id=\"30\">&lt;style&gt;@import \"data:,*%7bx:alert&#40;30&#41;)%7D\";&lt;/style&gt;//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"31\"><frameset onload=alert(31)>//[\"'`-->]]>]</div>",
@@ -473,19 +473,19 @@
   },
   {
     "payload": "<label dataformatas=\"html\" datasrc=\"#xss\" datafld=\"payload\"></label>//[\"'`-->]]>]</div>",
-    "expected": "<label \"html\" \"#xss\" \"payload\"></label>//[\"'`--&gt;]]>]</div>"
+    "expected": "<label dataformatas=\"html\" datasrc=\"#xss\" datafld=\"payload\"></label>//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"54\"><script>[{'a':Object.prototype.__defineSetter__('b',function(){alert(arguments[0])}),'b':['secret']}]</script>//[\"'`-->]]>]</div>",
     "expected": "<div id=\"54\">[{'a':Object.prototype.__defineSetter__('b',function(){alert&#40;arguments[0]&#41;}),'b':['secret']}]//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"55\"><video><source onerror=\"alert(55)\">//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"55\">&lt;video&gt;&lt; >//[\"'`--&>]</>"
+    "expected": "<div id=\"55\">&lt;video&gt;&lt; >//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"56\"><video onerror=\"alert(56)\"><source></source></video>//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"56\">< ><></></>//[\"'`--&gt;]]>]</div>"
+    "expected": "<div id=\"56\">&lt;video &gt;&lt;&gt;&lt;/>&lt;/video&gt;//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"57\"><b <script>alert(57)//</script>0</script></b>//[\"'`-->]]>]</div>",
@@ -553,7 +553,7 @@
   },
   {
     "payload": "<div id=\"74\"><a href=\"javascript:alert(74)\"><event-source src=\"data:application/x-dom-event-stream,Event:click%0Adata:XXX%0A%0A\" /></a>//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"74\"><a href=\"\"><event-source src=\"data:application/x-dom-event-stream,Event:click\ndata:XXX\n\n\" /></a>//[\"'`--&gt;]]>]</div>"
+    "expected": "<div id=\"74\"><a href=\"\"><event-source src=\"data:application/x-dom-event-stream,Event:click%0Adata:XXX%0A%0A\" /></a>//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"75\"><script<{alert(75)}/></script </>//[\"'`-->]]>]</div>",
@@ -585,7 +585,7 @@
   },
   {
     "payload": "<div id=\"82\"><?xml-stylesheet type=\"text/css\" href=\"data:,*%7bx:expression(write(2));%7d\"?>//[\"'`-->]]>]</div><div id=\"83\"><x:template xmlns:x=\"http://www.wapforum.org/2001/wml\"  x:ontimer=\"$(x:unesc)j$(y:escape)a$(z:noecs)v$(x)a$(y)s$(z)cript$x:alert(83)\"><x:timer value=\"1\"/></x:template>//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"82\">&lt;?xml-stylesheet type=\"text/css\" href=\"data:,*{x:write(2));}\"?&gt;//[\"'`--&gt;]]>]</div><div id=\"83\"><x:template xmlns:x=\"http://www.wapforum.org/2001/wml\"  x:><x:timer value=\"1\"/></x:template>//[\"'`--&gt;]]>]</div>"
+    "expected": "<div id=\"82\">&lt;?xml-stylesheet type=\"text/css\" href=\"data:,*%7bx:write(2));%7d\"?&gt;//[\"'`--&gt;]]>]</div><div id=\"83\"><x:template xmlns:x=\"http://www.wapforum.org/2001/wml\"  x:><x:timer value=\"1\"/></x:template>//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"84\"><x xmlns:ev=\"http://www.w3.org/2001/xml-events\" ev:event=\"load\" ev:handler=\"javascript:alert(84)//#x\"/>//[\"'`-->]]>]</div>",
@@ -601,7 +601,7 @@
   },
   {
     "payload": "<div id=\"88\"><svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\">\n<animation xlink:href=\"javascript:alert(88)\"/>\n<animation xlink:href=\"data:text/xml,%3Csvg xmlns='http://www.w3.org/2000/svg' onload='alert(88)'%3E%3C/svg%3E\"/>\n\n<image xlink:href=\"data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' onload='alert(88)'%3E%3C/svg%3E\"/>\n\n<foreignObject xlink:href=\"javascript:alert(88)\"/>\n<foreignObject xlink:href=\"data:text/xml,%3Cscript xmlns='http://www.w3.org/1999/xhtml'%3Ealert(88)%3C/script%3E\"/>\n\n</svg>//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"88\">&lt;svg :xlink=\"http://www.w3.org/1999/xlink\"&gt;\n<animation =\"data:text/xml,&lt;svg  ='alert&#40;88&#41;'&gt;&lt;/svg>\"/>\n\n<foreignObject =\"data:text/xml,alert&#40;88&#41;\"/>\n\n&lt;/svg&gt;//[\"'`--&gt;]]>]</div>"
+    "expected": "<div id=\"88\">&lt;svg :xlink=\"http://www.w3.org/1999/xlink\"&gt;\n<animation =\"data:text/xml,%3Csvg  >\n\n<image  ='http://www.w3.org/1999/xhtml'%3Ealert&#40;88&#41;%3C/script%3E\"/>\n\n&lt;/svg&gt;//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"89\"><svg xmlns=\"http://www.w3.org/2000/svg\">\n<set attributeName=\"onmouseover\" to=\"alert(89)\"/>\n<animate attributeName=\"onunload\" to=\"alert(89)\"/>\n</svg>//[\"'`-->]]>]</div>",
@@ -629,7 +629,7 @@
   },
   {
     "payload": "<div id=\"95\"><svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\">\n<feImage>\n<set attributeName=\"xlink:href\" to=\"data:image/svg+xml;charset=utf-8;base64,\nPHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciPjxzY3JpcHQ%2BYWxlcnQoMSk8L3NjcmlwdD48L3N2Zz4NCg%3D%3D\"/>\n</feImage>\n</svg>//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"95\">&lt;svg :xlink=\"http://www.w3.org/1999/xlink\"&gt;\n<feImage>\n<set attributeName=\"xlink:href\" to=\nPHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciPjxzY3JpcHQ+YWxlcnQoMSk8L3NjcmlwdD48L3N2Zz4NCg==\"/>\n</feImage>\n&lt;/svg&gt;//[\"'`--&gt;]]>]</div>"
+    "expected": "<div id=\"95\">&lt;svg :xlink=\"http://www.w3.org/1999/xlink\"&gt;\n<feImage>\n<set attributeName=\"xlink:href\" to=\nPHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciPjxzY3JpcHQ%2BYWxlcnQoMSk8L3NjcmlwdD48L3N2Zz4NCg%3D%3D\"/>\n</feImage>\n&lt;/svg&gt;//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"96\"><iframe src=mhtml:http://html5sec.org/test.html!xss.html></iframe>\n<iframe src=mhtml:http://html5sec.org/test.gif!xss.html></iframe>//[\"'`-->]]>]</div>",
@@ -661,11 +661,11 @@
   },
   {
     "payload": "<div id=\"103\"><script>history.pushState(0,0,'/i/am/somewhere_else');</script>//[\"'`-->]]>]</div><div id=\"104\"><svg xmlns=\"http://www.w3.org/2000/svg\" id=\"foo\">\n<x xmlns=\"http://www.w3.org/2001/xml-events\" event=\"load\" observer=\"foo\" handler=\"data:image/svg+xml,%3Csvg%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%0A%3Chandler%20xml%3Aid%3D%22bar%22%20type%3D%22application%2Fecmascript%22%3E alert(104) %3C%2Fhandler%3E%0A%3C%2Fsvg%3E%0A#bar\"/>\n</svg>//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"103\">history.pushState(0,0,'/i/am/somewhere_else');//[\"'`--&gt;]]>]</div><div id=\"104\">&lt;svg =\"http://www.w3.org/2001/xml-events\" event=\"load\" observer=\"foo\" handler=\"data:image/svg+xml,&lt;svg >\n<handler xml:id=\"bar\" type=\"application/ecmascript\"> alert&#40;104&#41; </handler>\n&lt;/svg&gt;\n#bar\"/>\n&lt;/svg&gt;//[\"'`--&gt;]]>]</div>"
+    "expected": "<div id=\"103\">history.pushState(0,0,'/i/am/somewhere_else');//[\"'`--&gt;]]>]</div><div id=\"104\">&lt;svg =\"http://www.w3.org/2001/xml-events\" event=\"load\" observer=\"foo\" handler=\"data:image/svg+xml,%3Csvg%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%0A%3Chandler%20xml%3Aid%3D%22bar%22%20type%3D%22application%2Fecmascript%22%3E alert&#40;104&#41; %3C%2Fhandler%3E%0A%3C%2Fsvg%3E%0A#bar\"/&gt;\n&lt;/svg&gt;//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"105\"><iframe src=\"data:image/svg-xml,%1F%8B%08%00%00%00%00%00%02%03%B3)N.%CA%2C(Q%A8%C8%CD%C9%2B%B6U%CA())%B0%D2%D7%2F%2F%2F%D7%2B7%D6%CB%2FJ%D77%B4%B4%B4%D4%AF%C8(%C9%CDQ%B2K%CCI-*%D10%D4%B4%D1%87%E8%B2%03\"></iframe>//[\"'`-->]]>]</div>",
-    "expected": "<div id=\"105\">&lt;iframe src=\"data:image/svg-xml,‹³)N.Ê,(Q¨ÈÍÉ+¶UÊ())°Ò×///×+7ÖË/J×7´´´ԯÈ(ÉÍQ²KÌI-*Ñ0Դчè²\"&gt;&lt;/iframe>//[\"'`--&gt;]]>]</div>"
+    "expected": "<div id=\"105\">&lt;iframe src=\"data:image/svg-xml,%1F%8B%B3)N.%CA%2C(Q%A8%C8%CD%C9%2B%B6U%CA())%B0%D2%D7%2F%2F%2F%D7%2B7%D6%CB%2FJ%D77%B4%B4%B4%D4%AF%C8(%C9%CDQ%B2K%CCI-*%D10%D4%B4%D1%87%E8%B2\"&gt;&lt;/iframe>//[\"'`--&gt;]]>]</div>"
   },
   {
     "payload": "<div id=\"106\"><img src onerror /\" '\"= alt=alert(106)//\">//[\"'`-->]]>]</div>",