From 49b1b8edc47528f1caf12a7251921fcf70edebf8 Mon Sep 17 00:00:00 2001 From: Lars Moelleken Date: Mon, 16 Apr 2018 01:14:31 +0200 Subject: [PATCH] [!]: don't edit the encoding, if no XSS was detected -> this will also fix issue #32 --- src/voku/helper/AntiXSS.php | 110 +++++++++++-------------------- tests/JsXssTest.php | 4 +- tests/LaravelSecurityTest.php | 4 +- tests/XssTest.php | 57 ++++++++-------- tests/fixtures/expect.json | 24 +++---- tests/fixtures/xss_v1_clean.html | 20 ++---- tests/fixtures/xss_v1_clean.svg | 32 ++++++++- 7 files changed, 117 insertions(+), 134 deletions(-) diff --git a/src/voku/helper/AntiXSS.php b/src/voku/helper/AntiXSS.php index 68814e1..d9c041a 100644 --- 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', + '<script>', + '</script>', ); /** @@ -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', + '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

XSS hash

- */ - 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 index 554e8ae..3676608 100644 --- 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('abcd', $this->security->xss_clean('abcd')); @@ -191,7 +191,7 @@ public function testFromJsXss() // HTML5新增实体编码 冒号: 换行 self::assertSame('', $this->security->xss_clean('')); self::assertSame('', $this->security->xss_clean('')); - self::assertSame("", $this->security->xss_clean('')); + self::assertSame("", $this->security->xss_clean('')); self::assertSame('', $this->security->xss_clean('')); self::assertSame('', $this->security->xss_clean('')); diff --git a/tests/LaravelSecurityTest.php b/tests/LaravelSecurityTest.php index f713f1b..aff1fb7 100644 --- a/tests/LaravelSecurityTest.php +++ b/tests/LaravelSecurityTest.php @@ -91,7 +91,7 @@ public function snippetProvider() ), array( '" => '<iframe name=alert(1) src="//somedomain?x=\',__defineSetter__(\'x\',eval),x=name,\'"></iframe>', "" => 'x = \'\',__defineSetter__(\'x\',alert),x=1,\'\';', // NoScript XSS filter bypass | 2015: http://blog.portswigger.net/2015/07/noscript-xss-filter-bypass.html - '">CLICKME 😃' => '">CLICKME 😃', // NoScript XSS filter bypass | 2015: https://twitter.com/0x6D6172696F/status/623081477002014720?s=02 + '">CLICKME 😃' => '">CLICKME 😃', // NoScript XSS filter bypass | 2015: https://twitter.com/0x6D6172696F/status/623081477002014720?s=02 '
aa
' => '
aa
', // IE | 2014: http://wooyun.org/bugs/wooyun-2014-068564 '
xss' => 'xss', // Firefox | 2007: https://bugzilla.mozilla.org/show_bug.cgi?id=369814 - '
  • Now click to execute arbitrary JS
  • ' => '
  • alert(document.domain)">">Now click to execute arbitrary JS
  • ', // Chrome 33 | 2015: view-source:https://html5sec.org/test/bypass + '
  • Now click to execute arbitrary JS
  • ' => '
  • Now click to execute arbitrary JS
  • ', // Chrome 33 | 2015: view-source:https://html5sec.org/test/bypass 'ipt>alert(1)ri' => '<svg>/<@/>alert(1)', @@ -235,7 +233,7 @@ public function testXssClean() '
    <' => '<style>@KeyFrames x{</style><div > <', // Chrome | 2016 | https://twitter.com/0x6D6172696F/status/669183179165720576 '
    ' => '<style>:target{zoom:2;transition:1s}</style><div id=x >', // https://twitter.com/cgvwzq/status/684316889221337088 'lose focus!click this!copy this!right click this!copy this!double click this!drag this!focus this!input here!press any key!press any key!press any key!click this!hover this!hover this!hover this!click this!paste here!0000' => 'lose focus!click this!copy this!right click this!copy this!double click this!drag this!focus this!input here!press any key!press any key!press any key!click this!hover this!hover this!hover this!click this!paste here!0000', // 2015 | http://brutelogic.com.br/blog/agnostic-event-handlers/ - '