summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--modules/gallery/controllers/permissions.php2
-rw-r--r--modules/gallery/controllers/simple_uploader.php1
-rw-r--r--modules/gallery/tests/Controller_Auth_Test.php245
-rw-r--r--modules/gallery/tests/controller_auth_data.txt17
-rw-r--r--modules/tag/controllers/tags.php2
-rw-r--r--modules/user/controllers/admin_users.php2
6 files changed, 267 insertions, 2 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..c4dc915b
--- /dev/null
+++ b/modules/gallery/tests/Controller_Auth_Test.php
@@ -0,0 +1,245 @@
+<?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();
+ 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) {
+ 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;
+ } else if ($open_braces == 0) {
+ $is_admin_controller = false;
+ $is_rest_controller = false;
+ }
+ $function = null;
+ } 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));
+
+ // 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));
+
+ 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);
+ }
+ }
+ }
+
+ // 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, "required"), $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"), $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 = $val;
+ }
+ return $this->_uses_input;
+ }
+
+ function checks_authorization($val=null) {
+ if ($val !== null) {
+ $this->_checks_authorization = $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..aabd2863
--- /dev/null
+++ 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");
generated by cgit v1.2.3 (git 2.46.0) at 2025-10-13 09:09:40 +0000