diff options
Diffstat (limited to 'modules')
| -rw-r--r-- | modules/gallery/controllers/permissions.php | 2 | ||||
| -rw-r--r-- | modules/gallery/controllers/simple_uploader.php | 1 | ||||
| -rw-r--r-- | modules/gallery/tests/Controller_Auth_Test.php | 252 | ||||
| -rw-r--r-- | modules/gallery/tests/controller_auth_data.txt | 40 | ||||
| -rw-r--r-- | modules/organize/controllers/organize.php | 4 | ||||
| -rw-r--r-- | modules/rss/controllers/rss.php | 4 | ||||
| -rw-r--r-- | modules/server_add/controllers/admin_server_add.php | 13 | ||||
| -rw-r--r-- | modules/tag/controllers/tags.php | 5 | ||||
| -rw-r--r-- | modules/user/controllers/admin_users.php | 2 |
9 files changed, 313 insertions, 10 deletions
diff --git a/modules/gallery/controllers/permissions.php b/modules/gallery/controllers/permissions.php index 5f4620b2..8d75862e 100644 --- a/modules/gallery/controllers/permissions.php +++ b/modules/gallery/controllers/permissions.php @@ -81,7 +81,7 @@ class Permissions_Controller extends Controller { } } - function _get_form($item) { + private function _get_form($item) { $view = new View("permissions_form.html"); $view->item = $item; $view->groups = ORM::factory("group")->find_all(); diff --git a/modules/gallery/controllers/simple_uploader.php b/modules/gallery/controllers/simple_uploader.php index 156d18ac..bc508319 100644 --- a/modules/gallery/controllers/simple_uploader.php +++ b/modules/gallery/controllers/simple_uploader.php @@ -32,6 +32,7 @@ class Simple_Uploader_Controller extends Controller { } public function start() { + access::verify_csrf(); batch::start(); } diff --git a/modules/gallery/tests/Controller_Auth_Test.php b/modules/gallery/tests/Controller_Auth_Test.php new file mode 100644 index 00000000..caf6d8f2 --- /dev/null +++ b/modules/gallery/tests/Controller_Auth_Test.php @@ -0,0 +1,252 @@ +<?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 Controller_Auth_Test extends Unit_Test_Case { + static $rest_methods = array("_index", "_show", "_form_edit", "_form_add", "_create", + "_update", "_delete"); + + static $rest_methods_with_csrf_check = array("_update", "_delete", "_create"); + + public function find_missing_auth_test() { + $found = array(); + $controllers = glob("*/*/controllers/*.php"); + $feeds = glob("*/*/helpers/*_rss.php"); + foreach (array_merge($controllers, $feeds) as $controller) { + if (preg_match("{modules/(gallery_)?unit_test/}", $controller)) { + continue; + } + + // List of all tokens without whitespace, simplifying parsing. + $tokens = array(); + foreach (token_get_all(file_get_contents($controller)) as $token) { + if (!is_array($token) || $token[0] != T_WHITESPACE) { + $tokens[] = $token; + } + } + + $is_admin_controller = false; + $is_rest_controller = false; + + $open_braces = 0; + $function = null; + for ($token_number = 0; $token_number < count($tokens); $token_number++) { + $token = $tokens[$token_number]; + + // Count braces. + // 1 open brace = in class context. + // 2 open braces = in function. + if (!is_array($token)) { + if ($token == "}") { + $open_braces--; + if ($open_braces == 1 && $function) { + $found[$controller][] = $function; + $function = null; + } else if ($open_braces == 0) { + $is_admin_controller = false; + $is_rest_controller = false; + } + } else if ($token == "{") { + $open_braces++; + } + } else { + // An array token + + if ($open_braces == 0 && $token[0] == T_EXTENDS) { + if (self::_token_matches(array(T_STRING, "Admin_Controller"), $tokens, $token_number + 1)) { + $is_admin_controller = true; + } else if (self::_token_matches(array(T_STRING, "REST_Controller"), $tokens, $token_number + 1)) { + $is_rest_controller = true; + } + } else if ($open_braces == 1 && $token[0] == T_FUNCTION) { + $line = $token[2]; + $name = ""; + // Search backwards to check visibility, + // "private function", or "private static function" + $previous = $tokens[$token_number - 1][0]; + $previous_2 = $tokens[$token_number - 2][0]; + $is_private = in_array($previous, array(T_PRIVATE, T_PROTECTED)) || + in_array($previous_2, array(T_PRIVATE, T_PROTECTED)); + $is_static = $previous == T_STATIC || $previous_2 == T_STATIC; + + // Search forward to get function name + do { + $token_number++; + if (self::_token_matches(array(T_STRING), $tokens, $token_number)) { + $token = $tokens[$token_number]; + $name = $token[1]; + break; + } + } while ($token_number < count($tokens)); + + $is_rss_feed = $name == "feed" && strpos(basename($controller), "_rss.php"); + + if ((!$is_static || $is_rss_feed) && + (!$is_private || + ($is_rest_controller && in_array($name, self::$rest_methods)))) { + $function = self::_function($name, $line, $is_admin_controller); + if ($is_rest_controller && in_array($name, self::$rest_methods_with_csrf_check)) { + $function->checks_csrf(true); + } + } + } + + // Check body of all public functions + // + // Authorization + // Require: access::required\( + // Authentication (CSRF token) + // [When using Input, $this->input, Forge] + // Require: ->validate() or access::verify_csrf\( + if ($function && $open_braces >= 2) { + if ($token[0] == T_STRING) { + if ($token[1] == "access" && + self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1) && + self::_token_matches(array(T_STRING), $tokens, $token_number + 2) && + in_array($tokens[$token_number + 2][1], array("forbidden", "required")) && + self::_token_matches("(", $tokens, $token_number + 3)) { + $token_number += 3; + $function->checks_authorization(true); + } else if ($token[1] == "access" && + self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1) && + self::_token_matches(array(T_STRING, "verify_csrf"), $tokens, $token_number + 2) && + self::_token_matches("(", $tokens, $token_number + 3)) { + $token_number += 3; + $function->checks_csrf(true); + } else if (in_array($token[1], array("Input", "Forge")) && + self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1)) { + $token_number++; + $function->uses_input(true); + } + } else if ($token[0] == T_VARIABLE) { + if ($token[1] == '$this' && + self::_token_matches(array(T_OBJECT_OPERATOR), $tokens, $token_number + 1) && + self::_token_matches(array(T_STRING, "input"), $tokens, $token_number + 2)) { + $token_number += 2; + $function->uses_input(true); + } + } else if ($token[0] == T_OBJECT_OPERATOR) { + if (self::_token_matches(array(T_STRING, "validate"), $tokens, $token_number + 1) && + self::_token_matches("(", $tokens, $token_number + 2)) { + $token_number += 2; + $function->checks_csrf(true); + } + } + } + } + } + } + + // Generate the report + $new = TMPPATH . "controller_auth_data.txt"; + $fd = fopen($new, "wb"); + ksort($found); + foreach ($found as $controller => $functions) { + $is_admin_controller = true; + foreach ($functions as $function) { + $is_admin_controller &= $function->is_admin_controller; + $flags = array(); + if ($function->uses_input() && !$function->checks_csrf()) { + $flags[] = "DIRTY_CSRF"; + } + if (!$function->is_admin_controller && !$function->checks_authorization()) { + $flags[] = "DIRTY_AUTH"; + } + + if (!$flags) { + // Don't print CLEAN instances + continue; + } + + fprintf($fd, "%-60s %-20s %s\n", + $controller, $function->name, implode("|", $flags)); + } + + if (strpos(basename($controller), "admin_") === 0 && !$is_admin_controller) { + fprintf($fd, "%-60s %-20s %s\n", + $controller, basename($controller), "NO_ADMIN_CONTROLLER"); + } + } + fclose($fd); + + // Compare with the expected report from our golden file. + $canonical = MODPATH . "gallery/tests/controller_auth_data.txt"; + exec("diff $canonical $new", $output, $return_value); + $this->assert_false( + $return_value, "Controller auth golden file mismatch. Output:\n" . implode("\n", $output) ); + } + + 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; + } + } + + static function _function($name, $line, $is_admin_controller) { + return new Controller_Auth_Test_Function($name, $line, $is_admin_controller); + } +} + +class Controller_Auth_Test_Function { + public $name; + public $line; + public $is_admin_controller = false; + private $_uses_input = false; + private $_checks_authorization = false; + private $_checks_csrf = false; + + function __construct($name, $line, $is_admin_controller) { + $this->name = $name; + $this->line = $line; + $this->is_admin_controller = $is_admin_controller; + } + + function uses_input($val=null) { + if ($val !== null) { + $this->_uses_input = (bool) $val; + } + return $this->_uses_input; + } + + function checks_authorization($val=null) { + if ($val !== null) { + $this->_checks_authorization = (bool) $val; + } + return $this->_checks_authorization; + } + + function checks_csrf($val=null) { + if ($val !== null) { + $this->_checks_csrf = $val; + } + return $this->_checks_csrf; + } +}
\ No newline at end of file diff --git a/modules/gallery/tests/controller_auth_data.txt b/modules/gallery/tests/controller_auth_data.txt new file mode 100644 index 00000000..fdf00c5e --- /dev/null +++ b/modules/gallery/tests/controller_auth_data.txt @@ -0,0 +1,40 @@ +modules/comment/controllers/admin_comments.php queue DIRTY_CSRF +modules/comment/controllers/comments.php _index DIRTY_CSRF +modules/comment/helpers/comment_rss.php feed DIRTY_AUTH +modules/digibug/controllers/digibug.php print_proxy DIRTY_CSRF|DIRTY_AUTH +modules/digibug/controllers/digibug.php close_window DIRTY_AUTH +modules/gallery/controllers/admin.php __call DIRTY_AUTH +modules/gallery/controllers/albums.php _show DIRTY_CSRF +modules/gallery/controllers/albums.php _form_add DIRTY_CSRF +modules/gallery/controllers/combined.php javascript DIRTY_AUTH +modules/gallery/controllers/combined.php css DIRTY_AUTH +modules/gallery/controllers/file_proxy.php __call DIRTY_CSRF|DIRTY_AUTH +modules/gallery/controllers/maintenance.php index DIRTY_AUTH +modules/gallery/controllers/rest.php __construct DIRTY_AUTH +modules/gallery/controllers/rest.php __call DIRTY_AUTH +modules/gallery/controllers/rest.php form_edit DIRTY_AUTH +modules/gallery/controllers/rest.php form_add DIRTY_AUTH +modules/gallery/controllers/rest.php _index DIRTY_AUTH +modules/gallery/controllers/rest.php _create DIRTY_AUTH +modules/gallery/controllers/rest.php _show DIRTY_AUTH +modules/gallery/controllers/rest.php _update DIRTY_AUTH +modules/gallery/controllers/rest.php _delete DIRTY_AUTH +modules/gallery/controllers/rest.php _form_add DIRTY_AUTH +modules/gallery/controllers/rest.php _form_edit DIRTY_AUTH +modules/gallery/controllers/simple_uploader.php start DIRTY_AUTH +modules/gallery/controllers/simple_uploader.php finish DIRTY_AUTH +modules/gallery/controllers/upgrader.php index DIRTY_AUTH +modules/gallery/controllers/welcome_message.php index DIRTY_AUTH +modules/rss/controllers/rss.php feed DIRTY_CSRF|DIRTY_AUTH +modules/search/controllers/search.php index DIRTY_CSRF|DIRTY_AUTH +modules/server_add/controllers/admin_server_add.php autocomplete DIRTY_CSRF +modules/server_add/controllers/server_add.php children DIRTY_CSRF +modules/tag/controllers/admin_tags.php index DIRTY_CSRF +modules/tag/controllers/tags.php _show DIRTY_CSRF|DIRTY_AUTH +modules/user/controllers/login.php ajax DIRTY_AUTH +modules/user/controllers/login.php auth_ajax DIRTY_AUTH +modules/user/controllers/login.php html DIRTY_AUTH +modules/user/controllers/login.php auth_html DIRTY_AUTH +modules/user/controllers/logout.php index DIRTY_CSRF|DIRTY_AUTH +modules/user/controllers/password.php reset DIRTY_AUTH +modules/user/controllers/password.php do_reset DIRTY_CSRF|DIRTY_AUTH diff --git a/modules/organize/controllers/organize.php b/modules/organize/controllers/organize.php index 259c94e7..08c80de3 100644 --- a/modules/organize/controllers/organize.php +++ b/modules/organize/controllers/organize.php @@ -45,9 +45,13 @@ class Organize_Controller extends Controller { access::verify_csrf(); $target_album = ORM::factory("item", $target_album_id); + access::required("view", $target_album); + access::required("add", $target_album); + foreach ($this->input->post("source_ids") as $source_id) { $source = ORM::factory("item", $source_id); if (!$source->contains($target_album)) { + access::required("edit", $source); item::move($source, $target_album); } } diff --git a/modules/rss/controllers/rss.php b/modules/rss/controllers/rss.php index b89bed40..015d6032 100644 --- a/modules/rss/controllers/rss.php +++ b/modules/rss/controllers/rss.php @@ -21,13 +21,13 @@ class Rss_Controller extends Controller { public static $page_size = 20; public function feed($module_id, $feed_id, $id=null) { - $page = $this->input->get("page", 1); + $page = (int) $this->input->get("page", 1); if ($page < 1) { url::redirect(url::merge(array("page" => 1))); } // Configurable page size between 1 and 100, default 20 - $page_size = max(1, min(100, $this->input->get("page_size", self::$page_size))); + $page_size = max(1, min(100, (int) $this->input->get("page_size", self::$page_size))); // Run the appropriate feed callback if (module::is_active($module_id)) { diff --git a/modules/server_add/controllers/admin_server_add.php b/modules/server_add/controllers/admin_server_add.php index fac2aa44..7cd82d60 100644 --- a/modules/server_add/controllers/admin_server_add.php +++ b/modules/server_add/controllers/admin_server_add.php @@ -34,15 +34,17 @@ class Admin_Server_Add_Controller extends Admin_Controller { $form = $this->_get_admin_form(); $paths = unserialize(module::get_var("server_add", "authorized_paths", "a:0:{}")); if ($form->validate()) { - if (is_readable($form->add_path->path->value)) { + if (is_link($form->add_path->path->value)) { + $form->add_path->path->add_error("is_symlink", 1); + } else if (! is_readable($form->add_path->path->value)) { + $form->add_path->path->add_error("not_readable", 1); + } else { $path = $form->add_path->path->value; $paths[$path] = 1; module::set_var("server_add", "authorized_paths", serialize($paths)); message::success(t("Added path %path", array("path" => $path))); server_add::check_config($paths); url::redirect("admin/server_add"); - } else { - $form->add_path->path->add_error("not_readable", 1); } } @@ -84,9 +86,10 @@ class Admin_Server_Add_Controller extends Admin_Controller { array("id" => "gServerAddAdminForm")); $add_path = $form->group("add_path"); $add_path->input("path")->label(t("Path"))->rules("required") - ->error_messages("not_readable", t("This directory is not readable by the webserver")); + ->error_messages("not_readable", t("This directory is not readable by the webserver")) + ->error_messages("is_symlink", t("Symbolic links are not allowed")); $add_path->submit("add")->value(t("Add Path")); return $form; } -}
\ No newline at end of file +} diff --git a/modules/tag/controllers/tags.php b/modules/tag/controllers/tags.php index c993e374..1bd6b3cc 100644 --- a/modules/tag/controllers/tags.php +++ b/modules/tag/controllers/tags.php @@ -22,7 +22,7 @@ class Tags_Controller extends REST_Controller { public function _show($tag) { $page_size = module::get_var("gallery", "page_size", 9); - $page = $this->input->get("page", "1"); + $page = (int) $this->input->get("page", "1"); $children_count = $tag->items_count(); $offset = ($page-1) * $page_size; @@ -43,6 +43,9 @@ class Tags_Controller extends REST_Controller { } public function _index() { + // Far from perfection, but at least require view permission for the root album + $album = ORM::factory("item", 1); + access::required("view", $album); print tag::cloud(30); } diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php index 521f82fa..0b748955 100644 --- a/modules/user/controllers/admin_users.php +++ b/modules/user/controllers/admin_users.php @@ -17,7 +17,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ -class Admin_Users_Controller extends Controller { +class Admin_Users_Controller extends Admin_Controller { public function index() { $view = new Admin_View("admin.html"); $view->content = new View("admin_users.html"); |