diff options
Diffstat (limited to 'modules/gallery')
| -rw-r--r-- | modules/gallery/helpers/p.php | 16 | ||||
| -rw-r--r-- | modules/gallery/libraries/I18n.php | 20 | ||||
| -rw-r--r-- | modules/gallery/libraries/MY_ORM.php | 4 | ||||
| -rw-r--r-- | modules/gallery/libraries/SafeString.php | 142 | ||||
| -rw-r--r-- | modules/gallery/tests/SafeString_Test.php | 111 | ||||
| -rw-r--r-- | modules/gallery/tests/Xss_Security_Test.php | 325 |
6 files changed, 532 insertions, 86 deletions
diff --git a/modules/gallery/helpers/p.php b/modules/gallery/helpers/p.php index 862c769b..e852c086 100644 --- a/modules/gallery/helpers/p.php +++ b/modules/gallery/helpers/p.php @@ -18,22 +18,12 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class p_Core { - private static $_purifier = null; static function clean($dirty_html) { - return html::specialchars($dirty_html); + return new SafeString($dirty_html); } + // Deprecated: Please use p::clean($var).purified_html() static function purify($dirty_html) { - if (empty(self::$_purifier)) { - require_once(dirname(__file__) . "/../lib/HTMLPurifier/HTMLPurifier.auto.php"); - $config = HTMLPurifier_Config::createDefault(); - foreach (Kohana::config('purifier') as $category => $key_value) { - foreach ($key_value as $key => $value) { - $config->set("$category.$key", $value); - } - } - self::$_purifier = new HTMLPurifier($config); - } - return self::$_purifier->purify($dirty_html); + return SafeString::of($dirty_html)->purified_html(); } } diff --git a/modules/gallery/libraries/I18n.php b/modules/gallery/libraries/I18n.php index 03a6d8f6..8dc42e04 100644 --- a/modules/gallery/libraries/I18n.php +++ b/modules/gallery/libraries/I18n.php @@ -84,6 +84,12 @@ class I18n_Core { /** * Translates a localizable message. + * + * Security: + * The returned string is safe for use in HTML (it contains a safe subset of HTML and + * interpolation parameters are converted to HTML entities). + * For use in JavaScript, please call ->for_js() on it. + * * @param $message String|array The message to be translated. E.g. "Hello world" * or array("one" => "One album", "other" => "%count albums") * @param $options array (optional) Options array for key value pairs which are used @@ -110,7 +116,7 @@ class I18n_Core { $entry = $this->interpolate($locale, $entry, $values); - return $entry; + return SafeString::of($entry)->mark_html_safe(); } private function lookup($locale, $message) { @@ -179,17 +185,19 @@ class I18n_Core { return is_array($message); } - private function interpolate($locale, $string, $values) { + private function interpolate($locale, $string, $key_values) { // TODO: Handle locale specific number formatting. // Replace x_y before replacing x. - krsort($values, SORT_STRING); + krsort($key_values, SORT_STRING); $keys = array(); - foreach (array_keys($values) as $key) { + $values = array(); + foreach ($key_values as $key => $value) { $keys[] = "%$key"; + $values[] = new SafeString($value); } - return str_replace($keys, array_values($values), $string); + return str_replace($keys, $values, $string); } private function pluralize($locale, $entry, $count) { @@ -414,4 +422,4 @@ class I18n_Core { return $count == 1 ? 'one' : 'other'; } } -}
\ No newline at end of file +} diff --git a/modules/gallery/libraries/MY_ORM.php b/modules/gallery/libraries/MY_ORM.php index de8adc1d..2c9ad1d7 100644 --- a/modules/gallery/libraries/MY_ORM.php +++ b/modules/gallery/libraries/MY_ORM.php @@ -43,6 +43,10 @@ class ORM extends ORM_Core { $this->original = clone $this; } + if ($value instanceof SafeString) { + $value = $value->unescaped(); + } + return parent::__set($column, $value); } diff --git a/modules/gallery/libraries/SafeString.php b/modules/gallery/libraries/SafeString.php new file mode 100644 index 00000000..53bcb27a --- /dev/null +++ b/modules/gallery/libraries/SafeString.php @@ -0,0 +1,142 @@ +<?php defined("SYSPATH") or die("No direct script access."); +/** + * Gallery - a web based photo album viewer and editor + * Copyright (C) 2000-2009 Bharat Mediratta + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or (at + * your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. + */ + +/** + * Safe string representation (regarding security - cross site scripting). + */ +class SafeString_Core { + private $_raw_string; + protected $_is_safe_html = false; + + private static $_purifier = null; + + /** Constructor */ + function __construct($string) { + if ($string instanceof SafeString) { + $this->_is_safe_html = $string->_is_safe_html; + $string = $string->unescaped(); + } + $this->_raw_string = (string) $string; + } + + /** + * Factory method returning a new SafeString instance for the given string. + */ + static function of($string) { + return new SafeString($string); + } + + /** + * Marks this string as safe to be used in HTML without any escaping. + */ + function mark_html_safe() { + $this->_is_safe_html = true; + return $this; + } + + /** + * Safe for use in HTML. + * @see #for_html() + */ + function __toString() { + if ($this->_is_safe_html) { + return $this->_raw_string; + } else { + return self::_escape_for_html($this->_raw_string); + } + } + + /** + * Safe for use in HTML. + * + * Example:<pre> + * <div><?= $php_var ?> + * </pre> + * @return the string escaped for use in HTML. + */ + function for_html() { + return $this; + } + + /** + * Safe for use in JavaScript. + * + * Example:<pre> + * <script type="text/javascript>" + * var some_js_var = "<?= $php_var->for_js() ?>"; + * </script> + * </pre> + * @return the string escaped for use in JavaScript. + */ + function for_js() { + return self::_escape_for_js($this->_raw_string); + } + + /** + * Safe for use HTML (purified HTML) + * + * Example:<pre> + * <div><?= $php_var->purified_html() ?> + * </pre> + * @return the string escaped for use in HTML. + */ + function purified_html() { + if ($this->_is_safe_html) { + return $this; + } else { + return SafeString::of(self::_purify_for_html($this->_raw_string), true); + } + } + + /** + * Returns the raw, unsafe string. Do not use lightly. + */ + function unescaped() { + return $this->_raw_string; + } + + // Escapes special HTML chars ("<", ">", "&", etc.) to HTML entities. + private static function _escape_for_html($dirty_html) { + return html::specialchars($dirty_html); + } + + // Escapes special chars (quotes, backslash, etc.) with a backslash sequence. + private static function _escape_for_js($string) { + // From Smarty plugins/modifier.escape.php + // Might want to be stricter here. + return strtr($string, + array('\\'=>'\\\\',"'"=>"\\'",'"'=>'\\"',"\r"=>'\\r',"\n"=>'\\n','</'=>'<\/')); + } + + // Purifies the string, removing any potentially malicious or unsafe HTML / JavaScript. + private static function _purify_for_html($dirty_html) { + if (empty(self::$_purifier)) { + require_once(dirname(__file__) . "/../lib/HTMLPurifier/HTMLPurifier.auto.php"); + $config = HTMLPurifier_Config::createDefault(); + foreach (Kohana::config('purifier') as $category => $key_value) { + foreach ($key_value as $key => $value) { + $config->set("$category.$key", $value); + } + } + self::$_purifier = new HTMLPurifier($config); + } + return self::$_purifier->purify($dirty_html); + } +} diff --git a/modules/gallery/tests/SafeString_Test.php b/modules/gallery/tests/SafeString_Test.php new file mode 100644 index 00000000..cdae3e99 --- /dev/null +++ b/modules/gallery/tests/SafeString_Test.php @@ -0,0 +1,111 @@ +<?php defined("SYSPATH") or die("No direct script access."); +/** + * Gallery - a web based photo album viewer and editor + * Copyright (C) 2000-2009 Bharat Mediratta + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or (at + * your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. + */ +class SafeString_Test extends Unit_Test_Case { + public function p_clean_returns_safestring_instance_test() { + $safe_string = p::clean("hello <p>world</p>"); + $this->assert_true($safe_string instanceof SafeString); + $this->assert_equal("hello <p>world</p>", + $safe_string->unescaped()); + } + + public function toString_escapes_for_html_test() { + $safe_string = new SafeString("hello <p>world</p>"); + $this->assert_equal("hello <p>world</p>", + $safe_string); + } + + public function toString_for_safe_string_test() { + $safe_string = new SafeString("hello <p>world</p>"); + $safe_string->mark_html_safe(); + $this->assert_equal("hello <p>world</p>", + $safe_string); + } + + public function for_html_test() { + $safe_string = new SafeString("hello <p>world</p>"); + $this->assert_equal("hello <p>world</p>", + $safe_string->for_html()); + } + + public function safestring_of_safestring_test() { + $safe_string = new SafeString("hello <p>world</p>"); + $safe_string_2 = new SafeString($safe_string); + $this->assert_true($safe_string_2 instanceof SafeString); + $raw_string = $safe_string_2->unescaped(); + $this->assert_false(is_object($raw_string)); + $this->assert_equal("hello <p>world</p>", $raw_string); + $this->assert_equal("hello <p>world</p>", $safe_string_2); + } + + public function for_js_test() { + $safe_string = new SafeString('"<em>Foo</em>\'s bar"'); + $js_string = $safe_string->for_js(); + $this->assert_equal('\\"<em>Foo<\\/em>\\\'s bar\\"', + $js_string); + } + + public function string_safestring_equality_test() { + $safe_string = new SafeString("hello <p>world</p>"); + $this->assert_equal("hello <p>world</p>", + $safe_string->unescaped()); + $escaped_string = "hello <p>world</p>"; + $this->assert_equal($escaped_string, $safe_string); + + $this->assert_true($escaped_string == $safe_string); + $this->assert_false($escaped_string === $safe_string); + $this->assert_false("meow" == $safe_string); + } + + public function of_test() { + $safe_string = SafeString::of("hello <p>world</p>"); + $this->assert_equal("hello <p>world</p>", $safe_string->unescaped()); + } + + public function of_safe_html_test() { + $safe_string = SafeString::of("hello <p>world</p>")->mark_html_safe(); + $this->assert_equal("hello <p>world</p>", $safe_string->for_html()); + } + + public function of_fluid_api_test() { + $escaped_string = SafeString::of("Foo's bar")->for_js(); + $this->assert_equal("Foo\\'s bar", $escaped_string); + } + + public function safestring_of_safestring_preserves_safe_status_test() { + $safe_string = SafeString::of("hello's <p>world</p>")->mark_html_safe(); + $safe_string_2 = new SafeString($safe_string); + $this->assert_equal("hello's <p>world</p>", $safe_string_2); + $this->assert_equal("hello\\'s <p>world<\\/p>", $safe_string_2->for_js()); + } + + public function safestring_of_safestring_preserves_html_safe_status_test() { + $safe_string = SafeString::of("hello's <p>world</p>") + ->mark_html_safe(); + $safe_string_2 = new SafeString($safe_string); + $this->assert_equal("hello's <p>world</p>", $safe_string_2); + $this->assert_equal("hello\\'s <p>world<\\/p>", $safe_string_2->for_js()); + } + + public function safestring_of_safestring_safe_status_override_test() { + $safe_string = new SafeString("hello <p>world</p>"); + $safe_string_2 = SafeString::of($safe_string)->mark_html_safe(); + $this->assert_equal("hello <p>world</p>", $safe_string_2); + } +} diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php index 9bde11dc..1d52237c 100644 --- a/modules/gallery/tests/Xss_Security_Test.php +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -19,87 +19,278 @@ */ class Xss_Security_Test extends Unit_Test_Case { public function find_unescaped_variables_in_views_test() { + $found = array(); foreach (glob("*/*/views/*.php") as $view) { - $expr = null; - $level = 0; - $php = 0; - $str = null; - $in_p_clean = 0; + // List of all tokens without whitespace, simplifying parsing. + $tokens = array(); foreach (token_get_all(file_get_contents($view)) as $token) { - if (false /* useful for debugging */) { - if (is_array($token)) { - printf("[$str] [$in_p_clean] %-15s %s\n", token_name($token[0]), $token[1]); - } else { - printf("[$str] [$in_p_clean] %-15s %s\n", "<char>", $token); - } - } - - // If we find a "(" after a "p::clean" then start counting levels of parens and assume - // that we're inside a p::clean() call until we find the matching close paren. - if ($token[0] == "(" && ($str == "p::clean" || $str == "p::purify")) { - $in_p_clean = 1; - } else if ($token[0] == "(" && $in_p_clean) { - $in_p_clean++; - } else if ($token[0] == ")" && $in_p_clean) { - $in_p_clean--; - } - - // Concatenate runs of strings for convenience, which we use above to figure out if we're - // inside a p::clean() call or not - if ($token[0] == T_STRING || $token[0] == T_DOUBLE_COLON) { - $str .= $token[1]; - } else { - $str = null; - } - - // Scan for any occurrences of < ? = $variable ? > and store it in $expr - if ($token[0] == T_OPEN_TAG_WITH_ECHO) { - $php++; - } else if ($php && $token[0] == T_CLOSE_TAG) { - $php--; - } else if ($php && $token[0] == T_VARIABLE) { - if (!$expr) { - $entry = array($token[2], $in_p_clean); - } - $expr .= $token[1]; - } else if ($expr) { - if ($token[0] == T_OBJECT_OPERATOR) { - $expr .= $token[1]; - } else if ($token[0] == T_STRING) { - $expr .= $token[1]; - } else if ($token == "(") { - $expr .= $token; - $level++; - } else if ($level > 0 && $token == ")") { - $expr .= $token; - $level--; - } else if ($level > 0) { - $expr .= is_array($token) ? $token[1] : $token; - } else { - $entry[] = $expr; - $found[$view][] = $entry; - $expr = null; - $entry = null; - } - } + if (!is_array($token) || ($token[0] != T_WHITESPACE)) { + $tokens[] = $token; + } + } + + $frame = null; + $script_block = 0; + $in_script_block = false; + + for ($token_number = 0; $token_number < count($tokens); $token_number++) { + $token = $tokens[$token_number]; + + // Are we in a <script> ... </script> block? + if (is_array($token) && $token[0] == T_INLINE_HTML) { + $inline_html = $token[1]; + // T_INLINE_HTML blocks can be split. Need to handle the case + // where one token has "<scr" and the next has "ipt" + while (self::_token_matches(array(T_INLINE_HTML), $tokens, $token_number + 1)) { + $token_number++; + $token = $tokens[$token_number]; + $inline_html .= $token[1]; + } + + if ($frame) { + $frame->expr_append($inline_html); + } + + // Note: This approach won't catch <script src="..."> blocks if the src + // URL is generated via < ? = url::site() ? > or some other PHP. + // Assume that all such script blocks with a src URL have an + // empty element body. + // But we'll catch closing tags for such blocks, so don't keep track + // of opening / closing tag count since it would be meaningless. + + // Handle multiple start / end blocks on the same line? + $opening_script_pos = $closing_script_pos = 0; + if (preg_match_all('{</script>}i', $inline_html, $matches, PREG_OFFSET_CAPTURE)) { + $last_match = array_pop($matches[0]); + if (is_array($last_match)) { + $closing_script_pos = $last_match[1]; + } else { + $closing_script_pos = $last_match; + } + } + if (preg_match('{<script\b[^>]*>}i', $inline_html, $matches, PREG_OFFSET_CAPTURE)) { + $last_match = array_pop($matches[0]); + if (is_array($last_match)) { + $opening_script_pos = $last_match[1]; + } else { + $opening_script_pos = $last_match; + } + } + if ($opening_script_pos != $closing_script_pos) { + $in_script_block = $opening_script_pos > $closing_script_pos; + } + } + + // Look and report each instance of < ? = ... ? > + if (!is_array($token)) { + // A single char token, e.g: ; ( ) + if ($frame) { + $frame->expr_append($token); + } + } 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); + } else if ($frame && $token[0] == T_CLOSE_TAG) { + // Store the < ? = ... ? > block that just ended here. + $found[$view][] = $frame; + $frame = null; + } else if ($frame && $token[0] == T_VARIABLE) { + $frame->expr_append($token[1]); + } else if ($frame && $token[0] == T_STRING) { + $frame->expr_append($token[1]); + // t() and t2() are special in that they're guaranteed to return a SafeString(). + if (in_array($token[1], array("t", "t2"))) { + if (self::_token_matches("(", $tokens, $token_number + 1)) { + $frame->is_safestring(true); + $frame->expr_append("("); + + $token_number++; + $token = $tokens[$token_number]; + } + } else if ($token[1] == "SafeString") { + // Looking for SafeString::of(... + if (self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1) && + self::_token_matches(array(T_STRING, "of"), $tokens, $token_number + 2) && + self::_token_matches("(", $tokens, $token_number + 3)) { + $frame->is_safestring(true); + $frame->expr_append("::of("); + + $token_number += 3; + $token = $tokens[$token_number]; + } + } else if ($token[1] == "json_encode") { + if (self::_token_matches("(", $tokens, $token_number + 1)) { + $frame->json_encode_called(true); + $frame->expr_append("("); + + $token_number++; + $token = $tokens[$token_number]; + } + } + } else if ($frame && $token[0] == T_OBJECT_OPERATOR) { + $frame->expr_append($token[1]); + + if (self::_token_matches(array(T_STRING), $tokens, $token_number + 1) && + in_array($tokens[$token_number + 1][1], + array("for_js", "for_html", "purified_html")) && + self::_token_matches("(", $tokens, $token_number + 2)) { + + $method = $tokens[$token_number + 1][1]; + $frame->expr_append("$method("); + + $token_number += 2; + $token = $tokens[$token_number]; + + if ("for_js" == $method) { + $frame->for_js_called(true); + } else if ("for_html" == $method) { + $frame->for_html_called(true); + } else if ("purified_html" == $method) { + $frame->purified_html_called(true); + } + } + } else if ($frame) { + $frame->expr_append($token[1]); + } } } - $canonical = MODPATH . "gallery/tests/xss_data.txt"; + // Generate the report. + /* + * States for uses of < ? = X ? >: + * JS_XSS: + * In <script> block + * X can be anything without calling ->for_js() + * UNKNOWN: + * Outside <script> block: + * X can be anything without a call to ->for_html() or ->purified_html() + * CLEAN: + * Outside <script> block: + * X = t() or t2() + * X = * and for_html() or purified_html() is called + * Inside <script> block: + * X = * with ->for_js() or json_encode(...) + */ $new = TMPPATH . "xss_data.txt"; $fd = fopen($new, "wb"); ksort($found); - foreach ($found as $view => $entries) { - foreach ($entries as $entry) { - fwrite($fd, - sprintf("%-60s %-3s %-5s %s\n", - $view, $entry[0], $entry[1] ? "" : "DIRTY", $entry[2])); + foreach ($found as $view => $frames) { + foreach ($frames as $frame) { + $state = "UNKNOWN"; + if ($frame->in_script_block()) { + $state = "JS_XSS"; + if ($frame->for_js_called() || $frame->json_encode_called()) { + $state = "CLEAN"; + } + } else { + if ($frame->is_safestring() || $frame->purified_html_called() || $frame->for_html_called()) { + $state = "CLEAN"; + } + } + fprintf($fd, "%-60s %-3s %-8s %s\n", + $view, $frame->line(), $state, $frame->expr()); } } fclose($fd); + exit; + // Compare with the expected report from our golden file. + $canonical = MODPATH . "gallery/tests/xss_data.txt"; exec("diff $canonical $new", $output, $return_value); $this->assert_false( $return_value, "XSS golden file mismatch. Output:\n" . implode("\n", $output) ); } + + private static function _create_frame($token, $in_script_block) { + return new Xss_Security_Test_Frame($token[2], $in_script_block); + } + + private static function _token_matches($expected_token, &$tokens, $token_number) { + if (!isset($tokens[$token_number])) { + return false; + } + + $token = $tokens[$token_number]; + + if (is_array($expected_token)) { + for ($i = 0; $i < count($expected_token); $i++) { + if ($expected_token[$i] != $token[$i]) { + return false; + } + } + return true; + } else { + return $expected_token == $token; + } + } +} + +class Xss_Security_Test_Frame { + private $_expr = ""; + private $_in_script_block = false; + private $_is_safestring = false; + private $_for_js_called = false; + private $_for_html_called = false; + private $_purified_html_called = false; + private $_json_encode_called = false; + private $_line; + + function __construct($line_number, $in_script_block) { + $this->_line = $line_number; + $this->in_script_block($in_script_block); + } + + function expr() { + return $this->_expr; + } + + function expr_append($append_value) { + return $this->_expr .= $append_value; + } + + function in_script_block($new_val=NULL) { + if ($new_val !== NULL) { + $this->_in_script_block = (bool) $new_val; + } + return $this->_in_script_block; + } + + function is_safestring($new_val=NULL) { + if ($new_val !== NULL) { + $this->_is_safestring = (bool) $new_val; + } + return $this->_is_safestring; + } + + function json_encode_called($new_val=NULL) { + if ($new_val !== NULL) { + $this->_json_encode_called = (bool) $new_val; + } + return $this->_json_encode_called; + } + + function for_js_called($new_val=NULL) { + if ($new_val !== NULL) { + $this->_for_js_called = (bool) $new_val; + } + return $this->_for_js_called; + } + + function for_html_called($new_val=NULL) { + if ($new_val !== NULL) { + $this->_for_html_called = (bool) $new_val; + } + return $this->_for_html_called; + } + + function purified_html_called($new_val=NULL) { + if ($new_val !== NULL) { + $this->_purified_html_called = (bool) $new_val; + } + return $this->_purified_html_called; + } + + function line() { + return $this->_line; + } } |