From a5670d8d708c35589a695640694199c7b026877b Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 30 May 2009 17:14:17 -0700 Subject: gate $can_edit and $can_add on whether or not we have an $item at all (fixes a bug where search doesn't render because it has no item). --- modules/gallery/helpers/gallery_menu.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_menu.php b/modules/gallery/helpers/gallery_menu.php index 0f0e676d..09c2d91a 100644 --- a/modules/gallery/helpers/gallery_menu.php +++ b/modules/gallery/helpers/gallery_menu.php @@ -28,8 +28,8 @@ class gallery_menu_Core { $item = $theme->item(); - $can_edit = access::can("edit", $item) || $is_admin; - $can_add = access::can("add", $item) || $is_admin; + $can_edit = $item && access::can("edit", $item) || $is_admin; + $can_add = $item && (access::can("add", $item) || $is_admin); if ($item && $can_edit || $can_add) { $menu->append($options_menu = Menu::factory("submenu") -- cgit v1.2.3 From ad81861c331f60ec8c19ea11e47e2826660fa142 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 31 May 2009 00:11:02 -0700 Subject: First pass at an XSS security test, along with the "p" helper which can clean HTML output. --- modules/gallery/helpers/p.php | 33 +++++++ modules/gallery/tests/Xss_Security_Test.php | 138 ++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 modules/gallery/helpers/p.php create mode 100644 modules/gallery/tests/Xss_Security_Test.php (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/p.php b/modules/gallery/helpers/p.php new file mode 100644 index 00000000..69032840 --- /dev/null +++ b/modules/gallery/helpers/p.php @@ -0,0 +1,33 @@ +purify($dirty_html)); + } + + function clean($dirty_html) { + // return $dirty_html; + return htmlentities($dirty_html, ENT_QUOTES); + // return Purify::instance()->purify($dirty_html); + } +} diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php new file mode 100644 index 00000000..22c4a767 --- /dev/null +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -0,0 +1,138 @@ + array(), "t2" => array()); + $token_number = 0; + // Filter out HTML / whitespace, and build a lookup for global function calls. + foreach ($raw_tokens as $token) { + if ((!is_array($token)) || (($token[0] != T_WHITESPACE) && ($token[0] != T_INLINE_HTML))) { + if (is_array($token)) { + if ($token[0] == T_STRING && in_array($token[1], array("t", "t2"))) { + $func_token_list[$token[1]][] = $token_number; + } + } + $tokens[] = $token; + $token_number++; + } + } + unset($raw_tokens); + + if (!empty($func_token_list["t"])) { + l10n_scanner::_parse_t_calls($tokens, $func_token_list["t"], $cache); + } + if (!empty($func_token_list["t2"])) { + l10n_scanner::_parse_plural_calls($tokens, $func_token_list["t2"], $cache); + } + } + + public function find_unescaped_variables_in_views_test() { + // foreach (glob("*/*/views/*.php") as $view) { + foreach (array("modules/search/views/search.html.php") as $view) { + $expr = null; + $line = null; + $level = 0; + $php = 0; + $str = null; + $in_p_clean = 0; + 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", "", $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") { + $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; + } + } + } + } + + $canonical = MODPATH . "gallery/tests/xss_data.txt"; + $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 %-9s %s\n", + $view, $entry[0], $entry[1] ? "CLEAN" : "NOT_CLEAN", $entry[2])); + } + } + fclose($fd); + + exec("diff $canonical $new", $output, $return_value); + $this->assert_false( + $return_value, "XSS golden file mismatch. Output:\n" . implode("\n", $output) ); + } +} -- cgit v1.2.3 From f9a741782da848c707ac0a122c35e86061a0fbb2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 31 May 2009 12:33:10 -0700 Subject: Switch to using html::specialchars() for cleaning. --- modules/gallery/helpers/p.php | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/p.php b/modules/gallery/helpers/p.php index 69032840..c3074c23 100644 --- a/modules/gallery/helpers/p.php +++ b/modules/gallery/helpers/p.php @@ -18,16 +18,7 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class p_Core { - static function attr($dirty_html) { - // return $dirty_html; - return htmlentities($dirty_html, ENT_QUOTES); - // return str_replace('"', '"', $dirty_html); - // return str_replace('"', '"', Purify::instance()->purify($dirty_html)); - } - function clean($dirty_html) { - // return $dirty_html; - return htmlentities($dirty_html, ENT_QUOTES); - // return Purify::instance()->purify($dirty_html); + return html::specialchars($dirty_html); } } -- cgit v1.2.3 From 181c97ef4b29bb3c68a6c9b5d2f8165e8b44ba29 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 31 May 2009 12:53:03 -0700 Subject: Relax the regex we use to extract the movie size so that it works with the new version of ffmpeg that I have on my dev box (ffmpeg 0.5-svn17737+3:0.svn20090303-1) --- modules/gallery/helpers/movie.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index 15225fe7..3aa40dc9 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -118,8 +118,7 @@ class movie_Core { $cmd = escapeshellcmd($ffmpeg) . " -i " . escapeshellarg($filename) . " 2>&1"; $result = `$cmd`; - if (preg_match("/Stream.*?Video:.*?(\d+)x(\d+).*\ +([0-9\.]+) (fps|tb).*/", - $result, $regs)) { + if (preg_match("/Stream.*?Video:.*?(\d+)x(\d+)/", $result, $regs)) { list ($width, $height) = array($regs[1], $regs[2]); } else { list ($width, $height) = array(0, 0); -- cgit v1.2.3