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

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


Commits:
2cc17154 by Espoir Baraka at 2026-08-19T11:13:13+02:00
[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

- - - - -


3 changed files:

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


Changes:

=====================================
lib/tikiaccesslib.php
=====================================
@@ -873,8 +873,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
@@ -882,7 +881,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']
@@ -891,7 +891,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(
@@ -1666,25 +1667,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'],
@@ -1697,9 +1684,7 @@ class TikiAccessLib extends TikiLib
             'httpOrigin' => $_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
=====================================
@@ -85,8 +85,8 @@ if (
 }
 $smarty->assign('secure_token', $secure_token);
 
-if (isset($_REQUEST["change"])) {
-    $access->checkCsrf();
+if (isset($_REQUEST["change"]) && $access->checkCsrf()) {
+    $changePasswordProperties = $bruteForceProperties();
     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()]);


=====================================
tiki-login.php
=====================================
@@ -297,6 +297,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
@@ -537,6 +541,10 @@ if ($isvalid && ($isOpenIdValid || $access->checkCsrf(null, null, null, null, nu
         }
     }
 } 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()]);



View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/2cc1715466f9de7490c272f93989be125f707c73

-- 
View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/2cc1715466f9de7490c272f93989be125f707c73
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