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

Espoir Baraka pushed to branch master at Tiki Wiki CMS Groupware / Tiki


Commits:
55382cc2 by Espoir Baraka at 2026-08-18T21:55:15+02:00
[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

- - - - -


3 changed files:

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


Changes:

=====================================
lib/tikiaccesslib.php
=====================================
@@ -884,8 +884,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
@@ -893,7 +892,9 @@ class TikiAccessLib extends TikiLib
     private function csrfPhpErrorLog($msg)
     {
         global $prefs;
-        error_log(PHP_EOL // @phpstan-ignore disallowedFunctions.errorLog (access violation reporting)
+        $ticket = $this->ticket ?: ($_POST['ticket'] ?? '');
+        error_log( // @phpstan-ignore disallowedFunctions.errorLog (access violation reporting)
+            PHP_EOL
             . '**** ' . tr('Start CSRF error from') . $_SERVER['SERVER_NAME'] . ' *****' . PHP_EOL
             . '  ' . $msg . PHP_EOL
             . '  site_security_timeout' . tr('preference:') . $prefs['site_security_timeout']
@@ -902,13 +903,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);
-        $get = count($_GET) ? json_encode($_GET, JSON_PRETTY_PRINT) : tr('empty');
-        $post = count($_POST) ? json_encode($_POST, JSON_PRETTY_PRINT) : tr('empty');
-        error_log( // @phpstan-ignore disallowedFunctions.errorLog (access violation reporting)
-            PHP_EOL
-            . '  $_GET: ' . $get . PHP_EOL
-            . '  $_POST: ' . $post . PHP_EOL
+            . (isset($_SERVER['REQUEST_METHOD']) ? '  REQUEST_METHOD: ' . $_SERVER['REQUEST_METHOD'] . PHP_EOL : '')
+            . '  ticket: ' . ($ticket !== '' ? $ticket : tr('empty')) . PHP_EOL
             . '**** ' . tr('End CSRF error from') . $_SERVER['SERVER_NAME'] . ' *****'
         );
     }
@@ -1700,25 +1696,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'],
@@ -1731,9 +1713,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
=====================================
@@ -88,8 +88,7 @@ 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', $changePasswordProperties)) {


=====================================
tiki-login.php
=====================================
@@ -368,6 +368,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
@@ -616,6 +620,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 - record bruteforce attempts
     if (($prefs['bruteforce_protection'] ?? 'n') === 'y') {
         $bruteForce->attempt('login', $bruteForceProperties());



View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/55382cc29282ea7c79aa252821c3ecab385b3b06

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