[TikiWiki-commits] [Git][tikiwiki/tiki][29.x] [FIX] TikiAccessLib: Refactor CSRF handling and remove unnecessary logging

Espoir Baraka pushed to branch 29.x at Tiki Wiki CMS Groupware / Tiki


Commits:
5285b8a5 by Espoir Baraka at 2026-08-19T16:10:43+02:00
[FIX] TikiAccessLib: Refactor CSRF handling and remove unnecessary logging
---
* [FIX] TikiAccessLib: Refactor CSRF handling and remove unnecessary logging
---
* [FIX] TikiAccessLib: Refactor CSRF handling and remove unnecessary logging
---
* [FIX] tiki-change_password: Enhance CSRF validation logic

(cherry picked from commit 6a363f6bad3d85420a8fc13ebe3fe1932ee71114)

* [FIX] TikiAccessLib: Refactor CSRF handling and remove unnecessary logging

- Introduced new properties for tracking CSRF ticket and origin status.
- Simplified CSRF validation logic by removing redundant logging.
- Updated error handling to set appropriate CSRF status messages.
- Enhanced clarity and maintainability of the CSRF-related methods.

(cherry picked from commit 08e57fef52f139f0267e813c18c5f1b2937d066f)

See merge request tikiwiki/tiki!10961

(cherry picked from commit 55382cc29282ea7c79aa252821c3ecab385b3b06)

See merge request tikiwiki/tiki!10978

(cherry picked from commit 2cc1715466f9de7490c272f93989be125f707c73)

See merge request tikiwiki/tiki!10986

- - - - -


3 changed files:

- lib/tikiaccesslib.php
- tiki-change_password.php
- tiki-login.php


Changes:

=====================================
lib/tikiaccesslib.php
=====================================
@@ -869,8 +869,7 @@ class TikiAccessLib extends TikiLib
 
     /**
      * Utility to compose and write the error message from CSRF errors to the server php error log, adding on
-     * certain information regarding the environment. Broken into two pieces since the the GET and POST
-     * parameters have the potential to exceed the character limit.
+     * certain information regarding the environment. Request body content is omitted to avoid logging secrets.
      *
      * @param $msg  string      Description of the specific error which will be placed first ahead of
      *                     the environmental information
@@ -878,7 +877,8 @@ class TikiAccessLib extends TikiLib
     private function csrfPhpErrorLog($msg)
     {
         global $prefs;
-        error_log(PHP_EOL
+        error_log(
+            PHP_EOL
             . '**** ' . tr('Start CSRF error from') . $_SERVER['SERVER_NAME'] . ' *****' . PHP_EOL
             . '  ' . $msg . PHP_EOL
             . '  site_security_timeout' . tr('preference:') . $prefs['site_security_timeout']
@@ -887,7 +887,8 @@ class TikiAccessLib extends TikiLib
             . (isset($_SERVER['REQUEST_URI']) ? '  REQUEST_URI: ' . $_SERVER['REQUEST_URI'] . PHP_EOL : '')
             . (isset($_SERVER['HTTP_ORIGIN']) ? '  HTTP_ORIGIN: ' . $_SERVER['HTTP_ORIGIN'] . PHP_EOL : '')
             . (isset($_SERVER['HTTP_REFERER']) ? '  HTTP_REFERER: ' . $_SERVER['HTTP_REFERER'] . PHP_EOL : '')
-            . (isset($_SERVER['REQUEST_METHOD']) ? '  REQUEST_METHOD: ' . $_SERVER['REQUEST_METHOD'] : '') . PHP_EOL);
+            . (isset($_SERVER['REQUEST_METHOD']) ? '  REQUEST_METHOD: ' . $_SERVER['REQUEST_METHOD'] : '') . PHP_EOL
+        );
         $get = count($_GET) ? json_encode($_GET, JSON_PRETTY_PRINT) : tr('empty');
         $post = count($_POST) ? json_encode($_POST, JSON_PRETTY_PRINT) : tr('empty');
         error_log(PHP_EOL
@@ -1670,25 +1671,11 @@ class TikiAccessLib extends TikiLib
      */
     private function csrfSystemLog($csrfErrorId)
     {
-        global $prefs, $user;
+        global $prefs;
 
-        $tikilib = TikiLib::lib('tiki');
         $logslib = TikiLib::lib('logs');
 
-        $redactPass = function (&$item, $key) {
-            if (str_contains($key, 'pass')) {
-                $item = '** ' . tr('redacted') . ' **';
-            }
-        };
-
-        $get = count($_GET) ? $_GET : tr('empty');
-        if (is_array($get)) {
-            array_walk($get, $redactPass);
-        }
-        $post = count($_POST) ? $_POST : tr('empty');
-        if (is_array($post)) {
-            array_walk($post, $redactPass);
-        }
+        $ticket = $this->ticket ?: ($_POST['ticket'] ?? '');
 
         $logCsrf = [
             'serverName' => $_SERVER['SERVER_NAME'],
@@ -1701,9 +1688,7 @@ class TikiAccessLib extends TikiLib
             'httpOrigin' => isset($_SERVER['HTTP_ORIGIN']) ? $_SERVER['HTTP_ORIGIN'] : null,
             'httpReferer' => $_SERVER['HTTP_REFERER'],
             'requestMethod' => $_SERVER['REQUEST_METHOD'],
-            'queryString' => $_SERVER['QUERY_STRING'] ?? tr('empty'),
-            'get' => $get,
-            'post' => $post
+            'ticket' => $ticket !== '' ? $ticket : tr('empty'),
         ];
         $logslib->add_action('CSRF Error', 'system', 'system', $csrfErrorId, '', '', '', '', '', '', $logCsrf);
     }


=====================================
tiki-change_password.php
=====================================
@@ -82,8 +82,21 @@ if (
 }
 $smarty->assign('secure_token', $secure_token);
 
-if (isset($_REQUEST["change"])) {
-    $access->checkCsrf();
+if (isset($_REQUEST["change"]) && $access->checkCsrf()) {
+    if (($prefs['bruteforce_protection'] ?? 'n') === 'y') {
+        if (! $bruteForce->isOperationAllowed('change_password', ['ip' => $tikilib->get_ip_address()])) {
+            $nextAllowedTime = $bruteForce->getNextAllowedTime('change_password', ['ip' => $tikilib->get_ip_address()]);
+            $waitTime = $nextAllowedTime - time();
+            if ($waitTime > 60) {
+                $waitMessage = sprintf(tra('Too many password change attempts. Please try again in %d minutes and %d seconds.'), floor($waitTime / 60), $waitTime % 60);
+            } else {
+                $waitMessage = sprintf(tra('Too many password change attempts. Please try again in %d seconds.'), $waitTime);
+            }
+            $smarty->assign('msg', $waitMessage);
+            $smarty->display('error.tpl');
+            die;
+        }
+    }
 
     // Verify authentication (oldpass, actpass, or token)
     $is_authenticated = false;


=====================================
tiki-login.php
=====================================
@@ -293,6 +293,10 @@ if (
         Feedback::errorAndDie(tra('An error occurred trying to login. Please contact the administrator.'), \Laminas\Http\Response::STATUS_CODE_500);
     }
 } else {
+    // Validate CSRF before processing credentials so forged requests never reach authentication.
+    // OpenID Connect is handled above and intentionally skips CSRF (redirect-based flow).
+    $access->checkCsrf(null, null, null, null, null, 'page');
+
     // Verify user is valid
     if ($twoFaPrevalidated) {
         // Password was already validated before the 2FA prompt; use a token instead of re-checking
@@ -531,7 +535,27 @@ if ($isvalid && ($isOpenIdValid || $access->checkCsrf(null, null, null, null, nu
             }
         }
     }
-} else {    // if ($isvalid) = false
+} else {
+    // Unsuccessful login (bad credentials or CSRF) - do not leave a partial user context
+    unset($user);
+    $smarty->assign('user', '');
+
+    // if ($isvalid) = false - check and record bruteforce attempts
+    if (($prefs['bruteforce_protection'] ?? 'n') === 'y') {
+        $isOperationAllowed = $bruteForce->isOperationAllowed('login', ['user' => $requestedUser, 'ip' => $tikilib->get_ip_address()]);
+        if ($requestedUser && ! $isOperationAllowed) {
+            $nextAllowedTime = $bruteForce->getNextAllowedTime('login', ['user' => $requestedUser, 'ip' => $tikilib->get_ip_address()]);
+            $waitTime = $nextAllowedTime - time();
+            if ($waitTime > 60) {
+                $waitMessage = sprintf(tra('Too many login attempts. Please try again in %d minutes and %d seconds.'), floor($waitTime / 60), $waitTime % 60);
+            } else {
+                $waitMessage = sprintf(tra('Too many login attempts. Please try again in %d seconds.'), $waitTime);
+            }
+            $smarty->assign('msg', $waitMessage);
+            $smarty->display('error.tpl');
+            die;
+        }
+    }
     // check if site is closed
     if ($prefs['site_closed'] === 'y') {
         unset($bypass_siteclose_check);



View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/5285b8a5d4a9a5d9f6937cd2d97f7171e0875ca3

-- 
View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/5285b8a5d4a9a5d9f6937cd2d97f7171e0875ca3
You're receiving this email because of your account on gitlab.com. Manage all notifications: https://gitlab.com/-/profile/notifications | Help: https://gitlab.com/help

_______________________________________________
TikiWiki-cvs mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tikiwiki-cvs