From 26f6d8192ffdfd0280987ec2b9df0305e983746d Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Mon, 31 Aug 2009 01:11:50 -0700 Subject: [PATCH] Adding XSS test for href="javascript: and onclick="..." --- modules/gallery/tests/Xss_Security_Test.php | 46 ++++++- modules/gallery/tests/xss_data.txt | 125 ++++++++++---------- 2 files changed, 104 insertions(+), 67 deletions(-) diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php index 0ba5a58775..1d1acce8ad 100644 --- a/modules/gallery/tests/Xss_Security_Test.php +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -33,6 +33,8 @@ public function find_unescaped_variables_in_views_test() { $script_block = 0; $in_script_block = false; $inline_html = ""; + $in_attribute_js_context = false; + $href_attribute_start = false; for ($token_number = 0; $token_number < count($tokens); $token_number++) { $token = $tokens[$token_number]; @@ -48,6 +50,8 @@ public function find_unescaped_variables_in_views_test() { $inline_html .= $token[1]; } + $inline_html = str_replace("\n", " ", $inline_html); + if ($frame) { $frame->expr_append($inline_html); } @@ -82,7 +86,23 @@ public function find_unescaped_variables_in_views_test() { } } - $href_attribute_start = preg_match('{href\s*=\s*[\'"]?\s*$}i', str_replace("\n", "", $inline_html)); + $href_attribute_start = preg_match('{\bhref\s*=\s*[\'"]?\s*$}i', $inline_html); + + $pos = false; + if ($in_attribute_js_context && ($pos = strpos($inline_html, $delimiter)) !== false) { + $in_attribute_js_context = false; + } + if (!$in_attribute_js_context) { + $pos = ($pos === false) ? 0 : $pos; + if (preg_match('{\bhref\s*=\s*(")javascript:[^"]*$}i', $inline_html, $matches, 0, $pos) || + preg_match("{\bhref\s*=\s*(')javascript:[^']*$}i", $inline_html, $matches, 0, $pos) || + preg_match("{\bon[a-z]+\s*=\s*(')[^']*$}i", $inline_html, $matches, 0, $pos) || + preg_match('{\bon[a-z]+\s*=\s*(")[^"]*$}i', $inline_html, $matches, 0, $pos)) { + $in_attribute_js_context = true; + $delimiter = $matches[1]; + $inline_html = ""; + } + } // Look and report each instance of < ? = ... ? > if (!is_array($token)) { @@ -92,7 +112,8 @@ public function find_unescaped_variables_in_views_test() { } } else if ($token[0] == T_OPEN_TAG_WITH_ECHO) { // No need for a stack here - assume < ? = cannot be nested. - $frame = self::_create_frame($token, $in_script_block, $href_attribute_start); + $frame = self::_create_frame($token, $in_script_block, + $href_attribute_start, $in_attribute_js_context); $href_attribute_start = false; } else if ($frame && $token[0] == T_CLOSE_TAG) { // Store the < ? = ... ? > block that just ended here. @@ -244,6 +265,8 @@ public function find_unescaped_variables_in_views_test() { * X can be anything without calling ->for_js() * At the start of a href= attribute * X = anything but a url method + * In href="javascript: or onclick="...": + * X = anything (manual review required) * DIRTY: * Outside