From 17254799d1069e9f67de14460264cda76395746f Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Tue, 15 Sep 2009 20:27:04 -0700 Subject: Initial skeleton of Controller_Auth code audit test (non functional). --- modules/gallery/tests/Controller_Auth_Test.php | 211 +++++++++++++++++++++++++ modules/gallery/tests/controller_auth_data.txt | 0 2 files changed, 211 insertions(+) create mode 100644 modules/gallery/tests/Controller_Auth_Test.php create mode 100644 modules/gallery/tests/controller_auth_data.txt (limited to 'modules') diff --git a/modules/gallery/tests/Controller_Auth_Test.php b/modules/gallery/tests/Controller_Auth_Test.php new file mode 100644 index 00000000..9927859b --- /dev/null +++ b/modules/gallery/tests/Controller_Auth_Test.php @@ -0,0 +1,211 @@ +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, "require"), $tokens, $token_number + 2) && + 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 == 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", $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 => $frames) { + foreach ($functions as $function) { + $flags = array(); + if ($function->uses_input() && !$function->checks_csrf()) { + $flags[] = "DIRTY_CSRF"; + } + if ($function->checks_authorization()) { + $flags[] = "DIRTY_AUTH"; + } + + if (!$flags) { + // Don't print CLEAN instances + continue; + } + + fprintf($fd, "%-60s %-20s %-21s\n", + $controller, $function->name, implode("|", $flags)); + } + } + 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) { + return new Controller_Auth_Test_Function($name, $line); + } +} + +class Controller_Auth_Test_Function { + public $name; + public $line; + private $_uses_input = false; + private $_checks_authorization = false; + private $_checks_csrf = false; + + function __construct($name, $line) { + $this->name = $name; + $this->line = $line; + } + + function uses_input($val=null) { + if ($val !== null) { + $this->_uses_input = $val; + } + return $this->_uses_input; + } + + function checks_authorization($val) { + if ($val !== null) { + $this->_checks_authorization = $val; + } + return $this->_checks_authorization; + } + + function checks_csrf($val) { + 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..e69de29b -- cgit v1.2.3 From 61bbe1d78c409dbc2d4af771146878f8f720959a Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Tue, 15 Sep 2009 21:03:23 -0700 Subject: First functional version of Controller_Auth_Test --- modules/gallery/tests/Controller_Auth_Test.php | 48 ++++++++++++++++++-------- 1 file changed, 33 insertions(+), 15 deletions(-) (limited to 'modules') diff --git a/modules/gallery/tests/Controller_Auth_Test.php b/modules/gallery/tests/Controller_Auth_Test.php index 9927859b..e3eb4eaf 100644 --- a/modules/gallery/tests/Controller_Auth_Test.php +++ b/modules/gallery/tests/Controller_Auth_Test.php @@ -29,6 +29,8 @@ class Controller_Auth_Test extends Unit_Test_Case { } } + $is_admin_controller = false; + $open_braces = 0; $function = null; for ($token_number = 0; $token_number < count($tokens); $token_number++) { @@ -38,10 +40,12 @@ class Controller_Auth_Test extends Unit_Test_Case { // 1 open brace = in class context. // 2 open braces = in function. if (!is_array($token)) { - if ($token == "{") { + if ($token == "}") { $open_braces--; - if ($function) { + if ($open_braces == 1 && $function) { $found[$controller][] = $function; + } else if ($open_braces == 0) { + $is_admin_controller = false; } $function = null; } else if ($token == "{") { @@ -50,7 +54,11 @@ class Controller_Auth_Test extends Unit_Test_Case { } else { // An array token - if ($open_braces == 1 && $token[0] == T_FUNCTION) { + 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 ($open_braces == 1 && $token[0] == T_FUNCTION) { $line = $token[2]; $name = ""; // Search backwards to check visibility, @@ -63,15 +71,15 @@ class Controller_Auth_Test extends Unit_Test_Case { // Search forward to get function name do { $token_number++; - if (self_::token_matches(array(T_STRING), $tokens, $token_number)) { + if (self::_token_matches(array(T_STRING), $tokens, $token_number)) { $token = $tokens[$token_number]; - $name = $tokens[1]; + $name = $token[1]; break; } } while ($token_number < count($tokens)); if (!$is_private) { - $function = self::_function($name, $line); + $function = self::_function($name, $line, $is_admin_controller); } } @@ -86,7 +94,7 @@ class Controller_Auth_Test extends Unit_Test_Case { 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, "require"), $tokens, $token_number + 2) && + self::_token_matches(array(T_STRING, "required"), $tokens, $token_number + 2) && self::_token_matches("(", $tokens, $token_number + 3)) { $token_number += 3; $function->checks_authorization(true); @@ -109,7 +117,7 @@ class Controller_Auth_Test extends Unit_Test_Case { $function->uses_input(true); } } else if ($token[0] == T_OBJECT_OPERATOR) { - if (self::_token_matches(array(T_STRING), "validate", $token_number + 1) && + 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); @@ -124,13 +132,16 @@ class Controller_Auth_Test extends Unit_Test_Case { $new = TMPPATH . "controller_auth_data.txt"; $fd = fopen($new, "wb"); ksort($found); - foreach ($found as $controller => $frames) { + 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->checks_authorization()) { + if (!$function->is_admin_controller && !$function->checks_authorization()) { $flags[] = "DIRTY_AUTH"; } @@ -142,6 +153,11 @@ class Controller_Auth_Test extends Unit_Test_Case { fprintf($fd, "%-60s %-20s %-21s\n", $controller, $function->name, implode("|", $flags)); } + + if (strpos(basename($controller), "admin_") === 0 && !$is_admin_controller) { + fprintf($fd, "%-60s %-20s %-21s\n", + $controller, basename($controller), "NO_ADMIN_CONTROLLER"); + } } fclose($fd); @@ -171,21 +187,23 @@ class Controller_Auth_Test extends Unit_Test_Case { } } - static function _function($name, $line) { - return new Controller_Auth_Test_Function($name, $line); + 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) { + 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) { @@ -195,14 +213,14 @@ class Controller_Auth_Test_Function { return $this->_uses_input; } - function checks_authorization($val) { + function checks_authorization($val=null) { if ($val !== null) { $this->_checks_authorization = $val; } return $this->_checks_authorization; } - function checks_csrf($val) { + function checks_csrf($val=null) { if ($val !== null) { $this->_checks_csrf = $val; } -- cgit v1.2.3 From e168e0dfae28bb56289b4debae8825c104ee69f9 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Tue, 15 Sep 2009 21:50:48 -0700 Subject: CSRF / auth fixes, golden data file checkpoint --- modules/gallery/controllers/permissions.php | 2 +- modules/gallery/controllers/simple_uploader.php | 1 + modules/gallery/tests/Controller_Auth_Test.php | 8 ++++++-- modules/gallery/tests/controller_auth_data.txt | 17 +++++++++++++++++ modules/tag/controllers/tags.php | 2 ++ modules/user/controllers/admin_users.php | 2 +- 6 files changed, 28 insertions(+), 4 deletions(-) (limited to 'modules') 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 index e3eb4eaf..50afae8f 100644 --- a/modules/gallery/tests/Controller_Auth_Test.php +++ b/modules/gallery/tests/Controller_Auth_Test.php @@ -21,6 +21,10 @@ class Controller_Auth_Test extends Unit_Test_Case { public function find_missing_auth_test() { $found = array(); foreach (glob("*/*/controllers/*.php") as $controller) { + if (strpos($controller, "modules/unit_test/") !== false) { + continue; + } + // List of all tokens without whitespace, simplifying parsing. $tokens = array(); foreach (token_get_all(file_get_contents($controller)) as $token) { @@ -150,12 +154,12 @@ class Controller_Auth_Test extends Unit_Test_Case { continue; } - fprintf($fd, "%-60s %-20s %-21s\n", + fprintf($fd, "%-60s %-20s %s\n", $controller, $function->name, implode("|", $flags)); } if (strpos(basename($controller), "admin_") === 0 && !$is_admin_controller) { - fprintf($fd, "%-60s %-20s %-21s\n", + fprintf($fd, "%-60s %-20s %s\n", $controller, basename($controller), "NO_ADMIN_CONTROLLER"); } } diff --git a/modules/gallery/tests/controller_auth_data.txt b/modules/gallery/tests/controller_auth_data.txt index e69de29b..aabd2863 100644 --- a/modules/gallery/tests/controller_auth_data.txt +++ b/modules/gallery/tests/controller_auth_data.txt @@ -0,0 +1,17 @@ +modules/comment/controllers/admin_comments.php queue DIRTY_CSRF +modules/digibug/controllers/digibug.php close_window DIRTY_AUTH +modules/gallery/controllers/combined.php javascript DIRTY_AUTH +modules/gallery/controllers/combined.php css DIRTY_AUTH +modules/gallery/controllers/maintenance.php index 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/user/controllers/login.php ajax DIRTY_AUTH +modules/user/controllers/login.php html DIRTY_AUTH diff --git a/modules/tag/controllers/tags.php b/modules/tag/controllers/tags.php index c993e374..f4f98090 100644 --- a/modules/tag/controllers/tags.php +++ b/modules/tag/controllers/tags.php @@ -43,6 +43,8 @@ class Tags_Controller extends REST_Controller { } public function _index() { + // Far from perfection, but at least require view permission for the root album + access::required("view", 1); 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"); -- cgit v1.2.3 From dc3d45e7607acce91253e44c29998b8797131f93 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Tue, 15 Sep 2009 22:01:59 -0700 Subject: Add exception for REST controllers (no fixes necessary). --- modules/gallery/tests/Controller_Auth_Test.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'modules') diff --git a/modules/gallery/tests/Controller_Auth_Test.php b/modules/gallery/tests/Controller_Auth_Test.php index 50afae8f..c4dc915b 100644 --- a/modules/gallery/tests/Controller_Auth_Test.php +++ b/modules/gallery/tests/Controller_Auth_Test.php @@ -18,6 +18,11 @@ * 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(); foreach (glob("*/*/controllers/*.php") as $controller) { @@ -34,6 +39,7 @@ class Controller_Auth_Test extends Unit_Test_Case { } $is_admin_controller = false; + $is_rest_controller = false; $open_braces = 0; $function = null; @@ -50,6 +56,7 @@ class Controller_Auth_Test extends Unit_Test_Case { $found[$controller][] = $function; } else if ($open_braces == 0) { $is_admin_controller = false; + $is_rest_controller = false; } $function = null; } else if ($token == "{") { @@ -61,6 +68,8 @@ class Controller_Auth_Test extends Unit_Test_Case { 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]; @@ -82,8 +91,11 @@ class Controller_Auth_Test extends Unit_Test_Case { } } while ($token_number < count($tokens)); - if (!$is_private) { + if (!$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); + } } } -- cgit v1.2.3 From 7608870537503ec571f45a175c8486d7945e7c63 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Tue, 15 Sep 2009 22:51:49 -0700 Subject: Controller auth / CSRF fixes --- modules/gallery/tests/Controller_Auth_Test.php | 19 +++++++++++-------- modules/gallery/tests/controller_auth_data.txt | 22 ++++++++++++++++++++++ modules/organize/controllers/organize.php | 4 ++++ modules/rss/controllers/rss.php | 4 ++-- modules/tag/controllers/tags.php | 3 ++- 5 files changed, 41 insertions(+), 11 deletions(-) (limited to 'modules') diff --git a/modules/gallery/tests/Controller_Auth_Test.php b/modules/gallery/tests/Controller_Auth_Test.php index c4dc915b..cd4abe07 100644 --- a/modules/gallery/tests/Controller_Auth_Test.php +++ b/modules/gallery/tests/Controller_Auth_Test.php @@ -26,7 +26,7 @@ class Controller_Auth_Test extends Unit_Test_Case { public function find_missing_auth_test() { $found = array(); foreach (glob("*/*/controllers/*.php") as $controller) { - if (strpos($controller, "modules/unit_test/") !== false) { + if (preg_match("{modules/(gallery_)?unit_test/}", $controller)) { continue; } @@ -54,11 +54,11 @@ class Controller_Auth_Test extends Unit_Test_Case { $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; } - $function = null; } else if ($token == "{") { $open_braces++; } @@ -80,6 +80,7 @@ class Controller_Auth_Test extends Unit_Test_Case { $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 { @@ -91,7 +92,9 @@ class Controller_Auth_Test extends Unit_Test_Case { } } while ($token_number < count($tokens)); - if (!$is_private || ($is_rest_controller && in_array($name, self::$rest_methods))) { + if (!$is_static && + (!$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); @@ -110,7 +113,8 @@ class Controller_Auth_Test extends Unit_Test_Case { 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, "required"), $tokens, $token_number + 2) && + 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); @@ -125,7 +129,7 @@ class Controller_Auth_Test extends Unit_Test_Case { $token_number++; $function->uses_input(true); } - } else if ($token == T_VARIABLE) { + } 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)) { @@ -152,7 +156,6 @@ class Controller_Auth_Test extends Unit_Test_Case { $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"; @@ -224,14 +227,14 @@ class Controller_Auth_Test_Function { function uses_input($val=null) { if ($val !== null) { - $this->_uses_input = $val; + $this->_uses_input = (bool) $val; } return $this->_uses_input; } function checks_authorization($val=null) { if ($val !== null) { - $this->_checks_authorization = $val; + $this->_checks_authorization = (bool) $val; } return $this->_checks_authorization; } diff --git a/modules/gallery/tests/controller_auth_data.txt b/modules/gallery/tests/controller_auth_data.txt index aabd2863..fcb977e4 100644 --- a/modules/gallery/tests/controller_auth_data.txt +++ b/modules/gallery/tests/controller_auth_data.txt @@ -1,8 +1,17 @@ modules/comment/controllers/admin_comments.php queue DIRTY_CSRF +modules/comment/controllers/comments.php _index DIRTY_CSRF +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 @@ -13,5 +22,18 @@ modules/gallery/controllers/rest.php _form_add 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/tag/controllers/tags.php b/modules/tag/controllers/tags.php index f4f98090..b9f2c61c 100644 --- a/modules/tag/controllers/tags.php +++ b/modules/tag/controllers/tags.php @@ -44,7 +44,8 @@ class Tags_Controller extends REST_Controller { public function _index() { // Far from perfection, but at least require view permission for the root album - access::required("view", 1); + $album = ORM::factory("item", 1); + access::required("view", $album); print tag::cloud(30); } -- cgit v1.2.3 From 39632c4689842b3e3bb0715c0e9be757149c257d Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Tue, 15 Sep 2009 23:01:26 -0700 Subject: Also check for rss feeds in controller auth check --- modules/gallery/tests/Controller_Auth_Test.php | 8 ++++++-- modules/gallery/tests/controller_auth_data.txt | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'modules') diff --git a/modules/gallery/tests/Controller_Auth_Test.php b/modules/gallery/tests/Controller_Auth_Test.php index cd4abe07..caf6d8f2 100644 --- a/modules/gallery/tests/Controller_Auth_Test.php +++ b/modules/gallery/tests/Controller_Auth_Test.php @@ -25,7 +25,9 @@ class Controller_Auth_Test extends Unit_Test_Case { public function find_missing_auth_test() { $found = array(); - foreach (glob("*/*/controllers/*.php") as $controller) { + $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; } @@ -92,7 +94,9 @@ class Controller_Auth_Test extends Unit_Test_Case { } } while ($token_number < count($tokens)); - if (!$is_static && + $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); diff --git a/modules/gallery/tests/controller_auth_data.txt b/modules/gallery/tests/controller_auth_data.txt index fcb977e4..fdf00c5e 100644 --- a/modules/gallery/tests/controller_auth_data.txt +++ b/modules/gallery/tests/controller_auth_data.txt @@ -1,5 +1,6 @@ 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 -- cgit v1.2.3 From f1887422f8b4ba68dc273fe6f7d3f1123681e89a Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Tue, 15 Sep 2009 23:07:41 -0700 Subject: Stricter input handling (cast to int) --- modules/tag/controllers/tags.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules') diff --git a/modules/tag/controllers/tags.php b/modules/tag/controllers/tags.php index b9f2c61c..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; -- cgit v1.2.3 From 5490057480f17e5810cf8b9e558769ebd74d4b27 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Wed, 16 Sep 2009 12:27:13 -0700 Subject: When editing tags in place, and there is a validation error, highlight the tag with a red border and show a statust message. This fixes ticket: #779 --- modules/tag/controllers/admin_tags.php | 15 ++++++++++----- modules/tag/js/tag.js | 6 ++++++ 2 files changed, 16 insertions(+), 5 deletions(-) (limited to 'modules') diff --git a/modules/tag/controllers/admin_tags.php b/modules/tag/controllers/admin_tags.php index 8b8dde21..3301566b 100644 --- a/modules/tag/controllers/admin_tags.php +++ b/modules/tag/controllers/admin_tags.php @@ -81,15 +81,20 @@ class Admin_Tags_Controller extends Admin_Controller { kohana::show_404(); } - $form = tag::get_rename_form($tag); - $valid = $form->validate(); + //Don't use a form as the form is dynamically created in the js + $post = new Validation($_POST); + $post->add_rules("name", "required", "length[1,64]"); + $valid = $post->validate(); if ($valid) { - $new_name = $form->rename_tag->inputs["name"]->value; + $new_name = $this->input->post("name"); $new_tag = ORM::factory("tag")->where("name", $new_name)->find(); if ($new_tag->loaded) { - $form->rename_tag->inputs["name"]->add_error("in_use", 1); + $error_msg = "There is already a tag with that name"; $valid = false; } + } else { + $error_msg = $post->errors(); + $error_msg = $error_msg[0]; } if ($valid) { @@ -110,7 +115,7 @@ class Admin_Tags_Controller extends Admin_Controller { } else { print json_encode( array("result" => "error", - "form" => $form->__toString())); + "message" => $error_msg)); } } } diff --git a/modules/tag/js/tag.js b/modules/tag/js/tag.js index aaae9e72..d656da36 100644 --- a/modules/tag/js/tag.js +++ b/modules/tag/js/tag.js @@ -19,6 +19,7 @@ function ajaxify_tag_form() { function closeEditInPlaceForms() { // closes currently open inplace edit forms if ($("#gRenameTagForm").length) { + $("#gEditErrorMessage").remove(); var li = $("#gRenameTagForm").parent(); $("#gRenameTagForm").parent().html($("#gRenameTagForm").parent().data("revert")); li.height(""); @@ -66,6 +67,11 @@ function editInPlace(element) { $("#gTag-" + data.tag_id).text(data.new_tagname); // update tagname console.log(data); window.location.reload(); + } else if (data.result == "error") { + $("#gRenameTagForm #name") + .css("border", "2px solid red") + .focus(); + $("#gTagAdmin").before("

" + data.message + "

"); } } }); -- cgit v1.2.3