Reported Bug - Expired Security Token

cbeasley · March 2, 2021, 10:21pm

I made a post about a bug but am unsure how to ensure it’s listed as a bug. Any advice?

playcock · March 11, 2021, 12:59pm

You might have the best of luck reporting the issue in the Dolibarr GitHub project.

cbeasley · March 11, 2021, 9:49pm

===========

Bug-fix
modified:   admin/modules.php
modified:   core/ajax/constantonoff.php
modified:   main.inc.php
modified:   user/perms.php
==========

DIFF

diff --git a/htdocs/admin/modules.php b/htdocs/admin/modules.php
index 56b6e6e1f0…8b1a8ea15a 100644
— a/htdocs/admin/modules.php
+++ b/htdocs/admin/modules.php
@@ -29,7 +29,7 @@
*/

if (!defined(‘CSRFCHECK_WITH_TOKEN’)) {
define('CSRFCHECK_WITH_TOKEN', '1'); // Force use of CSRF protection with tokens even for GET
* ```
  define('CSRFCHECK_WITH_TOKEN', '0'); // Force use of CSRF protection with tokens even for GET
}

require ‘…/main.inc.php’;
diff --git a/htdocs/core/ajax/constantonoff.php b/htdocs/core/ajax/constantonoff.php
index a530bea092…751aca0202 100644
— a/htdocs/core/ajax/constantonoff.php
+++ b/htdocs/core/ajax/constantonoff.php
@@ -26,7 +26,7 @@ if (!defined(‘NOREQUIREHTML’)) define(‘NOREQUIREHTML’, ‘1’);
if (!defined(‘NOREQUIREAJAX’)) define(‘NOREQUIREAJAX’, ‘1’);
if (!defined(‘NOREQUIRESOC’)) define(‘NOREQUIRESOC’, ‘1’);
if (!defined(‘NOREQUIRETRAN’)) define(‘NOREQUIRETRAN’, ‘1’);
-if (!defined(‘CSRFCHECK_WITH_TOKEN’)) define(‘CSRFCHECK_WITH_TOKEN’, ‘1’); // Token is required even in GET mode
+if (!defined(‘CSRFCHECK_WITH_TOKEN’)) define(‘CSRFCHECK_WITH_TOKEN’, ‘0’); // Token is required even in GET mode

require ‘…/…/main.inc.php’;
require_once DOL_DOCUMENT_ROOT.’/core/lib/admin.lib.php’;
diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php
index 78ed3d82bd…7758465420 100644
— a/htdocs/main.inc.php
+++ b/htdocs/main.inc.php
@@ -427,8 +427,9 @@ if (!defined(‘NOTOKENRENEWAL’))
//$dolibarr_nocsrfcheck=1;
// Check token
if ((!defined(‘NOCSRFCHECK’) && empty($dolibarr_nocsrfcheck) && !empty($conf->global->MAIN_SECURITY_CSRF_WITH_TOKEN))
|| defined('CSRFCHECK_WITH_TOKEN'))     // Check validity of token, only if option MAIN_SECURITY_CSRF_WITH_TOKEN enabled or if constant CSRFCHECK_WITH_TOKEN is set into page
* ```
  || ( defined('CSRFCHECK_WITH_TOKEN') && ! empty(CSRFCHECK_WITH_TOKEN) ) )       // Check validity of token, only if option MAIN_SECURITY_CSRF_WITH_TOKEN enabled or if constant CSRFCHECK_WITH_TOKEN is set into page
{
+
// Check all cases that need a token (all POST actions, all actions and mass actions on pages with CSRFCHECK_WITH_TOKEN set, all sensitive GET actions)
if ($_SERVER[‘REQUEST_METHOD’] == ‘POST’ ||
((GETPOSTISSET(‘action’) || GETPOSTISSET(‘massaction’)) && defined(‘CSRFCHECK_WITH_TOKEN’)) ||
diff --git a/htdocs/user/perms.php b/htdocs/user/perms.php
index 68238b63c2…5fa2b47e38 100644
— a/htdocs/user/perms.php
+++ b/htdocs/user/perms.php
@@ -26,7 +26,7 @@
      \brief          Page to set permission of a user record
*/

-if (!defined(‘CSRFCHECK_WITH_TOKEN’)) define(‘CSRFCHECK_WITH_TOKEN’, ‘1’); // Force use of CSRF protection with tokens even for GET
+if (!defined(‘CSRFCHECK_WITH_TOKEN’)) define(‘CSRFCHECK_WITH_TOKEN’, ‘0’); // Force use of CSRF protection with tokens even for GET

require ‘…/main.inc.php’;
require_once DOL_DOCUMENT_ROOT.’/core/lib/usergroups.lib.php’;

I have created the following bug report in the GIT project.

github.com/Dolibarr/dolibarr

Reported Bug - Expired Security Token

opened 09:45PM - 11 Mar 21 UTC

closed 09:05PM - 05 Feb 23 UTC

carmelchas

Works for me / Can't reproduce Bug

# Bug Based on documentation I have read the feature for CSRF TOKEN verificatio…n is not complete. There was a report that stated upgrading to version 13.0.1 fixes the problem in the screenshot below, but it did not. There was also a post that stated setting $dolibarr_nocsrfcheck to ‘1’ in conf/conf.php fixes the problem, but it does not. ## Environment - **Version**: 13.0.1 ## Expected and actual behavior Based on documentation I have read the feature for CSRF TOKEN verification is not complete. There was a report that stated upgrading to version 13.0.1 fixes the problem in the screenshot below, but it did not. There was also a post that stated setting $dolibarr_nocsrfcheck to ‘1’ in conf/conf.php fixes the problem, but it does not. The following bug-fix provides the list of files and the differences I made to resolve the issue. In a nutshell the problem is: A. Multiple files define the constant ‘CSRFCHECK_WITH_TOKEN’ as ‘1’. B. The main.inc.php asserts CSRF CHECK logic whenever ‘CSRFCHECK_WITH_TOKEN’ is defined and DOES NOT consider the value for the constant. In a nutshell my temporary solution is: A. Everywhere ‘CSRFCHECK_WITH_TOKEN’ is defined change the setting to ‘0’. B. Update the main.inc.php conditions for applying the CSRF logic to additionally require that the value of CSRFCHECK_WITH_TOKEN is not empty. (@see DIFF below) ## Steps to reproduce the behavior Try to deactivate/activate a module and an error occurs. Previous solutions do not work. ## Bug-fix - Files modified: admin/modules.php modified: core/ajax/constantonoff.php modified: main.inc.php modified: user/perms.php ## DIFF diff --git a/htdocs/admin/modules.php b/htdocs/admin/modules.php index 56b6e6e1f0…8b1a8ea15a 100644 — a/htdocs/admin/modules.php +++ b/htdocs/admin/modules.php @@ -29,7 +29,7 @@ */ if (!defined(‘CSRFCHECK_WITH_TOKEN’)) { define('CSRFCHECK_WITH_TOKEN', '1'); // Force use of CSRF protection with tokens even for GET define('CSRFCHECK_WITH_TOKEN', '0'); // Force use of CSRF protection with tokens even for GET } require ‘…/main.inc.php’; diff --git a/htdocs/core/ajax/constantonoff.php b/htdocs/core/ajax/constantonoff.php index a530bea092…751aca0202 100644 — a/htdocs/core/ajax/constantonoff.php +++ b/htdocs/core/ajax/constantonoff.php @@ -26,7 +26,7 @@ if (!defined(‘NOREQUIREHTML’)) define(‘NOREQUIREHTML’, ‘1’); if (!defined(‘NOREQUIREAJAX’)) define(‘NOREQUIREAJAX’, ‘1’); if (!defined(‘NOREQUIRESOC’)) define(‘NOREQUIRESOC’, ‘1’); if (!defined(‘NOREQUIRETRAN’)) define(‘NOREQUIRETRAN’, ‘1’); -if (!defined(‘CSRFCHECK_WITH_TOKEN’)) define(‘CSRFCHECK_WITH_TOKEN’, ‘1’); // Token is required even in GET mode +if (!defined(‘CSRFCHECK_WITH_TOKEN’)) define(‘CSRFCHECK_WITH_TOKEN’, ‘0’); // Token is required even in GET mode require ‘…/…/main.inc.php’; require_once DOL_DOCUMENT_ROOT.’/core/lib/admin.lib.php’; diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php index 78ed3d82bd…7758465420 100644 — a/htdocs/main.inc.php +++ b/htdocs/main.inc.php @@ -427,8 +427,9 @@ if (!defined(‘NOTOKENRENEWAL’)) //$dolibarr_nocsrfcheck=1; // Check token if ((!defined(‘NOCSRFCHECK’) && empty($dolibarr_nocsrfcheck) && !empty($conf->global->MAIN_SECURITY_CSRF_WITH_TOKEN)) || defined('CSRFCHECK_WITH_TOKEN')) // Check validity of token, only if option MAIN_SECURITY_CSRF_WITH_TOKEN enabled or if constant CSRFCHECK_WITH_TOKEN is set into page || ( defined('CSRFCHECK_WITH_TOKEN') && ! empty(CSRFCHECK_WITH_TOKEN) ) ) // Check validity of token, only if option MAIN_SECURITY_CSRF_WITH_TOKEN enabled or if constant CSRFCHECK_WITH_TOKEN is set into page { + // Check all cases that need a token (all POST actions, all actions and mass actions on pages with CSRFCHECK_WITH_TOKEN set, all sensitive GET actions) if ($_SERVER[‘REQUEST_METHOD’] == ‘POST’ || ((GETPOSTISSET(‘action’) || GETPOSTISSET(‘massaction’)) && defined(‘CSRFCHECK_WITH_TOKEN’)) || diff --git a/htdocs/user/perms.php b/htdocs/user/perms.php index 68238b63c2…5fa2b47e38 100644 — a/htdocs/user/perms.php +++ b/htdocs/user/perms.php @@ -26,7 +26,7 @@ \brief Page to set permission of a user record */ -if (!defined(‘CSRFCHECK_WITH_TOKEN’)) define(‘CSRFCHECK_WITH_TOKEN’, ‘1’); // Force use of CSRF protection with tokens even for GET +if (!defined(‘CSRFCHECK_WITH_TOKEN’)) define(‘CSRFCHECK_WITH_TOKEN’, ‘0’); // Force use of CSRF protection with tokens even for GET require ‘…/main.inc.php’; require_once DOL_DOCUMENT_ROOT.’/core/lib/usergroups.lib.php’; ## [Attached files](https://help.github.com/articles/issue-attachments) (Screenshots, screencasts, dolibarr.log, debugging informations…) ![Dolibarr_CSRF_Token_Bug](https://user-images.githubusercontent.com/445543/110859512-25b5de00-8281-11eb-8549-47efaa57fb13.png)