Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security Token crash site if using old modules #16096

Closed
choybe opened this issue Jan 29, 2021 · 31 comments
Closed

Security Token crash site if using old modules #16096

choybe opened this issue Jan 29, 2021 · 31 comments
Labels
Bug This is a bug (something does not work as expected)

Comments

@choybe
Copy link

choybe commented Jan 29, 2021

Bug

With reference to the bug report: #16085 (comment)
Here now a follow up on the V13 incompatibility with additional modules not prepared for V13

Expected and actual behavior

In contrary to former Dolibarr version V13 don't tolerate any more installing and activating additional modules not prepared for V13.
They are not only not accepted. They crash the site the message Security Token expired. It is then no more possible to change anything at the site, not even deactivating via UI the modules.

Steps to reproduce the behavior

I installed a V13 version from the scratch and added an additional module max. version V12, (In this case with the DroppDown Menu, It was possible. But after the module was enabled. It was no more possible t o add or remove any module (not even the V13 inherent modules). Same is happening with other additional modules not yet ready for V13.

For now this bug can only be resolved with the deletion of the modules entries at database level (eg via phpmyadmin).
I see it essential that either the code of V13 do not allow to activate additional modules not compatible with V13 and/or goving an explicit a warning to the user, that only own or purchased modules from dolistore compatible with v13 can be activated.

@choybe choybe added the Bug This is a bug (something does not work as expected) label Jan 29, 2021
@choybe choybe changed the title Security Token crash site if using module Security Token crash site if using old modules Jan 29, 2021
@JESSTOFUNK
Copy link

Hey Friend. I have the same Issue. As I said on the other post you already close, I have two different installations (diferent servers).

They have the same configurations, I have the TOKEN error everywhere on the production server, but I do not have it on the TRAINING server, I do not have multycompany, but I have MAIN_FEATURES_LEVEL in 2 and the same external modules in both, anyways, the problem is only in my Production SITE.
Captura1
Captura2
Captura3

@JESSTOFUNK
Copy link

Is a downgrade from V13 to V12 possible?

@frederic34
Copy link
Member

did you try with MAIN_SECURITY_CSRF_WITH_TOKEN set to 0

@choybe
Copy link
Author

choybe commented Jan 30, 2021

I made yesterday several tests and can say for now. The bug has nothing to do with the server, the MAIN_SECURITY_CSRF_WITH_TOKEN or the MAIN_FEATURES_LEVEL It makes no difference how they are set. The security token expired message keep the same: But I could find out so far:
When you enable a lot of built-in modules you can get the message, but it doesn't crash the site. You just have to enable them again, and then they pass through. This behavior I had in a newly installed version. In an updated version from v12 not (both on same server).
But it is different with additional modules not yet up to date to v12, Most of them pass through without breaking the site, unless they are not fully functionally (eg multicompany), But in my case I had one additional modules that crashed the site (DropDown Menu). Once enabled I all admin activities where blocked by the security token message. Similar probably with the above mentioned Zen FusionMaps.
For the v13 version this means, something in the configuration of v13 provoke this security token expired error message in some cases even with the inherent built-in modules under certain circumstances and with additional it can happen that the provoke a crash the site, It is therefore strongly recommended not to update a v12 version with additional modules enabled. First all additional modules still not compatible with v13 have to be disabled before update and then after update step by step enabled again to test their conformity with v13 In case the site crash there is no way to disable the non-conform module via UI. The incompatible module has then to be disabled/deleted with your database tools (eg terminal or phpmyadmin).
However: I hope a lot the Dolibarr developers will find soon a solution for this bug.

@hregis
Copy link
Contributor

hregis commented Jan 30, 2021

@choybe I did tests with V13 and I don't have all these problems by activating the CSRF token, even with Multicompany!

@hregis
Copy link
Contributor

hregis commented Jan 30, 2021

@JESSTOFUNK What is you externals modules ? You try without this modules ?

@daraelmin
Copy link
Contributor

I have similar issue, when trying to go back on search page or list.PHP and using the "rerurn to last page" of firefox.

@choybe
Copy link
Author

choybe commented Jan 30, 2021

I just were getting a feedback from the DropDown menu developer. On his test site all seems compatible with v13. But on my site it crashes the site. - I can therefore only conclude that the bug is anywhere on v13 in the definition or refresh of on the security token.
What is also strange to me that I get a security token message even on built in modules, but after reload the side the message disappear, This looks to me like something going wrong with the refresh of the security token after setup modifications.

@AXeL-dev
Copy link
Contributor

AXeL-dev commented Jan 30, 2021

I think this have something to do with the CSRF token validation, & more exactly on this part of code:

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
{
// 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')) ||
in_array(GETPOST('action', 'aZ09'), array('add', 'addtimespent', 'update', 'install', 'delete', 'deletefilter', 'deleteoperation', 'deleteprof', 'deletepayment', 'confirm_create_user', 'confirm_create_thirdparty', 'confirm_reject_check')))
{
if (!GETPOSTISSET('token')) {
if (GETPOST('uploadform', 'int')) {
dol_syslog("--- Access to ".$_SERVER["PHP_SELF"]." refused. File size too large.");
$langs->loadLangs(array("errors", "install"));
print $langs->trans("ErrorFileSizeTooLarge").' ';
print $langs->trans("ErrorGoBackAndCorrectParameters");
die;
} else {
dol_syslog("--- Access to ".$_SERVER["PHP_SELF"]." refused by CSRFCHECK_WITH_TOKEN protection. Token not provided.");
if (defined('CSRFCHECK_WITH_TOKEN')) {
print "Access to a page that needs a token (constant CSRFCHECK_WITH_TOKEN is defined) is refused by CSRF protection in main.inc.php. Token not provided.\n";
} else {
print "Access to this page this way (POST method or GET with a sensible value for 'action' parameter) is refused by CSRF protection in main.inc.php. Token not provided.\n";
print "If you access your server behind a proxy using url rewriting and the parameter is provided by caller, you might check that all HTTP header are propagated (or add the line \$dolibarr_nocsrfcheck=1 into your conf.php file or MAIN_SECURITY_CSRF_WITH_TOKEN to 0 into setup).\n";
}
die;
}
}
}
if (GETPOSTISSET('token') && GETPOST('token', 'alpha') != $_SESSION['token'])
{
dol_syslog("--- Access to ".$_SERVER["PHP_SELF"]." refused due to invalid token, so we disable POST and some GET parameters - referer=".$_SERVER['HTTP_REFERER'].", action=".GETPOST('action', 'aZ09').", _GET|POST['token']=".GETPOST('token', 'alpha').", _SESSION['token']=".$_SESSION['token'], LOG_WARNING);
//print 'Unset POST by CSRF protection in main.inc.php.'; // Do not output anything because this create problems when using the BACK button on browsers.
setEventMessages('SecurityTokenHasExpiredSoActionHasBeenCanceledPleaseRetry', null, 'warnings');
//if ($conf->global->MAIN_FEATURES_LEVEL >= 1) setEventMessages('Unset POST and GET params by CSRF protection in main.inc.php (Token provided was not generated by the previous page).'."<br>\n".'$_SERVER[REQUEST_URI] = '.$_SERVER['REQUEST_URI'].' $_SERVER[REQUEST_METHOD] = '.$_SERVER['REQUEST_METHOD'].' GETPOST(token) = '.GETPOST('token', 'alpha').' $_SESSION[token] = '.$_SESSION['token'], null, 'warnings');
$savid = ((int) $_POST['id']);
unset($_POST);
//unset($_POST['action']); unset($_POST['massaction']);
//unset($_POST['confirm']); unset($_POST['confirmmassaction']);
unset($_GET['confirm']);
unset($_GET['action']);
unset($_GET['confirmmassaction']);
unset($_GET['massaction']);
$_POST['id'] = ((int) $savid);
}
}

The session token weirdly doesn't match with the GETPOST token.

As a temporary workaround, you can edit the main.inc.php file & search for the following line (in my case it was on line 442):

if (GETPOSTISSET('token') && GETPOST('token', 'alpha') != $_SESSION['token'])

then add this line just before it:

$_GET['token'] = $_SESSION['token']; // Tmp workaround for https://github.com/Dolibarr/dolibarr/issues/16096

Like so:

image

🚧 Keep in mind that this is only a temporary workaround until the issue get fixed by the Dolibarr team.

@daraelmin
Copy link
Contributor

Hi,

I'm not sure but it seems to me that the logical test doesn't match the comment in dolibarr/htdocs/main.inc.php Lines 429 to 431. I mean whats if MAIN_SECURITY_CSRF_WITH_TOKEN is defined but disabled ?

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`

@AXeL-dev
Copy link
Contributor

AXeL-dev commented Jan 30, 2021

Hi @daraelmin, i thought about that too but in the end i figured out that the logical test is correct, the comment is just not clear enough. Below are the cases when the logical test will fail:

(
! defined('NOCSRFCHECK')
+ // if NOCSRFCHECK is defined we exit the sub-condition
&&
empty($dolibarr_nocsrfcheck)
+ // if $dolibarr_nocsrfcheck is not empty we exit the sub-condition
&&
! empty ($conf->global->MAIN_SECURITY_CSRF_WITH_TOKEN)
+ // if $conf->global->MAIN_SECURITY_CSRF_WITH_TOKEN is empty (which means it's disabled) we exit the sub-condition again
)
+ // if one of the sub-conditions above fails
||
defined('CSRFCHECK_WITH_TOKEN')
+ // and if CSRFCHECK_WITH_TOKEN is not defined we completely exit the logical test

if we wrap up all together, it means that MAIN_SECURITY_CSRF_WITH_TOKEN have to be enabled + NOCSRFCHECK not defined + $dolibarr_nocsrfcheck empty or CSRFCHECK_WITH_TOKEN constant defined.

Edit:

whats if MAIN_SECURITY_CSRF_WITH_TOKEN is defined but disabled ?

if it's disabled then its value will be 0 & ! empty (0) is equal to false, which has the same return value as ! empty(undefined).

@daraelmin
Copy link
Contributor

Right ! I confused isset and empty ... beginner error, sorry.

Anyway.

May the problem come from the upgrade of the getpost() function?
Since v13, alpha in getpost is the same as alphanothml ('alpha'=Same than alphanohtml since v13 * 'alphanohtml'=check there is no html content and no " and no ../ * )
How is the tocken set ?

@AXeL-dev
Copy link
Contributor

AXeL-dev commented Jan 30, 2021

May the problem come from the upgrade of the getpost() function?

i don't think so, the problem is from token mismatch, i didn't go deep further in my investigation but if you simply add an:

echo GETPOST('token', 'alpha') . ' // ' . $_SESSION['token'];

before the following line:

if (GETPOSTISSET('token') && GETPOST('token', 'alpha') != $_SESSION['token'])

you'll see that the token value from GETPOST is different from the one stored in the session, which means that the token sent by the get or post method is not well generated or maybe outdated, i also saw that the session token is renewed each time the main.inc.php file get called so the logic of renewing the token just before the comparison is maybe the source of the issue (but i'm still not sure about that).

@choybe
Copy link
Author

choybe commented Jan 30, 2021

Hi @AXeL-dev and other. I can confirm your workaround #issuecomment-770215709 is working well. No more trouble with security token expired. Thanks a lot.,

@JESSTOFUNK
Copy link

@hregis I had some external modules, my application became useless, the message appeared even when I try to create a quote, agenda event or invoices, the token message didn't let me do nothing, so I was not able to disable the modules . I had to go the hard way, I created a brand new installation with V13 and this time, I do not have multi-company, I don't have external modules and I have MAIN_FEATURES_LEVEL in 0. The Token message still appears randomly some times, and it's becoming more recurrent but I do not have any of the items we thought was affecting, so I think eventually it is going to be useless again, but at least it lets me retry and at the second try it saves changes. The curious thing is that my TRAINING APPLICATION is not getting the message and I have FEATURES LEVEL 2 and all the extra modules.

@choybe Which solution are you running from @AXeL-dev?

@hregis
Copy link
Contributor

hregis commented Jan 31, 2021

@JESSTOFUNK @choybe
I think that it can come from Ajax calls in parallel which modify the token and suddenly it no longer corresponds to the original token of the call

@choybe
Copy link
Author

choybe commented Jan 31, 2021

from my side I can just say: With the solution of @AXeL-dev there are no more security token warnings when enabling or disabling any modules built-in, experimental, or provided from dolistore, even with those who are out of date (en my case max v 10, 11, 12)
So I think it has nothing to do with the MAIN_FEATURES_LEVEL or any not distribution inherent added module. The conclusion and work around of @AXeL-dev is, - due to my opinion -, the right way to debug this issue. Here the dolibarr-Team is demanded to fix this bug in the DB core.

@daraelmin
Copy link
Contributor

daraelmin commented Jan 31, 2021

hi @hregis,

I wonder if your solution is the good one cause I've found a new commit 4c9bf1d witch advice to use NOTOKENRENEWAL when using ajax.

  • All your Ajax services must contains such a line at begin of file: if (!defined('NOTOKENRENEWAL')) define('NOTOKENRENEWAL', '1'); // Disables token renewal

Cheers

@AXeL-dev
Copy link
Contributor

AXeL-dev commented Feb 3, 2021

@eldy @hregis is there any way to provide a fix in hurry? please

Edit: adding if (!defined('NOTOKENRENEWAL')) define('NOTOKENRENEWAL', '1'); to ajax files on external modules is not a wise solution, since this affects many modules, in addition that this issue also happens on modules enable/disable which has nothing to do with modules code (i guess).

@choybe
Copy link
Author

choybe commented Feb 21, 2021

I just updates to DB 13.01 - The bug continue - stiil needs to be solved with next version.
For now I will continue with the workaround at main inc.php of @AXeL-dev here at #issuecomment-770215709
as some other changes were made in version 13.01 the workaround has to be added at line 456.
So far for me the only solution that works for me. I hope the core team find soon an official and better solution.

@BB2A-Anthony
Copy link
Contributor

PR is here #16124

@choybe
Copy link
Author

choybe commented Feb 23, 2021

@eldy : not resolved with #16085
the bug persist still with the message of the missing security token , when trying to enable any internal or external module.

@Rekagi
Copy link

Rekagi commented Feb 26, 2021

@eldy : not resolved with #16085
the bug persist still with the message of the missing security token , when trying to enable any internal or external module.

100% true !

made the changes @eldy posted in the FIX. The modules do not work. Can't turn them on or off and pop up message is still there.

@daraelmin
Copy link
Contributor

The last merge 5095e7f should fix all cases

@frederic34
Copy link
Member

you can make this change https://github.com/Dolibarr/dolibarr/pull/16371/files to see in dolibarr.log which file reclaim a new token

@bluethumb
Copy link

bluethumb commented Mar 8, 2021

I have one 13 ( upgraded ) on my local cpu and one on ext. cpu installed 12->13.0.1
And it helped to have $dolibarr_nocsrfcheck=1 on both systems, and I do not see that many _GET[token] be set true the system even with MAIN_SECURITY_CSRF_WITH_TOKEN= 1

@eldy
Copy link
Member

eldy commented Mar 12, 2021

If, after adding this log, suggested by frederic24, you see in the dolibarr.log an ajax file or a js or a css page asking for a new token, it means you find a bug into this file. There is no such known files in v13.0.1 but a lot of external modules are still bugged and make the token renewal when they should not. Uninstall such modules. If it is notbpossible, try ti remove colpletely rhe directory of external module found into /custom dir.

@choybe
Copy link
Author

choybe commented Mar 20, 2021

I can confirm the suggestion of @eldy with db 13.02 : uninstalling all external modules helps and deleting them in htdocs/custom. To reinstall the external modules their compability it has to be tested one by one.

@BebZ
Copy link

BebZ commented May 25, 2021

Hello @eldy , if I understand well what you mean :
all the little customization work of our favourite ERP will be "locked" untill we modify every form of our custom modules ?
If not, it means we have to "erase" / "adapt" ?
It means weeks of very painful work... for which reason ?
Thanks to confirm what we all fear for v13.

@DATUC
Copy link
Contributor

DATUC commented Aug 23, 2021

I think this have something to do with the CSRF token validation, & more exactly on this part of code:

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
{
// 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')) ||
in_array(GETPOST('action', 'aZ09'), array('add', 'addtimespent', 'update', 'install', 'delete', 'deletefilter', 'deleteoperation', 'deleteprof', 'deletepayment', 'confirm_create_user', 'confirm_create_thirdparty', 'confirm_reject_check')))
{
if (!GETPOSTISSET('token')) {
if (GETPOST('uploadform', 'int')) {
dol_syslog("--- Access to ".$_SERVER["PHP_SELF"]." refused. File size too large.");
$langs->loadLangs(array("errors", "install"));
print $langs->trans("ErrorFileSizeTooLarge").' ';
print $langs->trans("ErrorGoBackAndCorrectParameters");
die;
} else {
dol_syslog("--- Access to ".$_SERVER["PHP_SELF"]." refused by CSRFCHECK_WITH_TOKEN protection. Token not provided.");
if (defined('CSRFCHECK_WITH_TOKEN')) {
print "Access to a page that needs a token (constant CSRFCHECK_WITH_TOKEN is defined) is refused by CSRF protection in main.inc.php. Token not provided.\n";
} else {
print "Access to this page this way (POST method or GET with a sensible value for 'action' parameter) is refused by CSRF protection in main.inc.php. Token not provided.\n";
print "If you access your server behind a proxy using url rewriting and the parameter is provided by caller, you might check that all HTTP header are propagated (or add the line \$dolibarr_nocsrfcheck=1 into your conf.php file or MAIN_SECURITY_CSRF_WITH_TOKEN to 0 into setup).\n";
}
die;
}
}
}
if (GETPOSTISSET('token') && GETPOST('token', 'alpha') != $_SESSION['token'])
{
dol_syslog("--- Access to ".$_SERVER["PHP_SELF"]." refused due to invalid token, so we disable POST and some GET parameters - referer=".$_SERVER['HTTP_REFERER'].", action=".GETPOST('action', 'aZ09').", _GET|POST['token']=".GETPOST('token', 'alpha').", _SESSION['token']=".$_SESSION['token'], LOG_WARNING);
//print 'Unset POST by CSRF protection in main.inc.php.'; // Do not output anything because this create problems when using the BACK button on browsers.
setEventMessages('SecurityTokenHasExpiredSoActionHasBeenCanceledPleaseRetry', null, 'warnings');
//if ($conf->global->MAIN_FEATURES_LEVEL >= 1) setEventMessages('Unset POST and GET params by CSRF protection in main.inc.php (Token provided was not generated by the previous page).'."<br>\n".'$_SERVER[REQUEST_URI] = '.$_SERVER['REQUEST_URI'].' $_SERVER[REQUEST_METHOD] = '.$_SERVER['REQUEST_METHOD'].' GETPOST(token) = '.GETPOST('token', 'alpha').' $_SESSION[token] = '.$_SESSION['token'], null, 'warnings');
$savid = ((int) $_POST['id']);
unset($_POST);
//unset($_POST['action']); unset($_POST['massaction']);
//unset($_POST['confirm']); unset($_POST['confirmmassaction']);
unset($_GET['confirm']);
unset($_GET['action']);
unset($_GET['confirmmassaction']);
unset($_GET['massaction']);
$_POST['id'] = ((int) $savid);
}
}

The session token weirdly doesn't match with the GETPOST token.

As a temporary workaround, you can edit the main.inc.php file & search for the following line (in my case it was on line 442):

if (GETPOSTISSET('token') && GETPOST('token', 'alpha') != $_SESSION['token'])

then add this line just before it:

$_GET['token'] = $_SESSION['token']; // Tmp workaround for https://github.com/Dolibarr/dolibarr/issues/16096

Like so:

image

🚧 Keep in mind that this is only a temporary workaround until the issue get fixed by the Dolibarr team.

Tks @AXeL-dev this help solve the token issue on my test lab

@clebeinfo
Copy link

dolibarr solution... mv to trash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is a bug (something does not work as expected)
Projects
None yet
Development

Successfully merging a pull request may close this issue.