Dolibarr / dolibarr

Dolibarr ERP CRM is a modern software package to manage your company or foundation's activity (contacts, suppliers, invoices, orders, stocks, agenda, accounting, ...). it's an open source Web application (written in PHP) designed for businesses of any sizes, foundations and freelancers.
https://www.dolibarr.org
GNU General Public License v3.0
5.29k stars 2.74k forks source link

Security Token crash site if using old modules #16096

Closed choybe closed 3 years ago

choybe commented 3 years ago

Bug

With reference to the bug report: https://github.com/Dolibarr/dolibarr/issues/16085#issuecomment-768865819 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.

JESSTOFUNK commented 3 years ago

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 commented 3 years ago

Is a downgrade from V13 to V12 possible?

frederic34 commented 3 years ago

did you try with MAIN_SECURITY_CSRF_WITH_TOKEN set to 0

choybe commented 3 years ago

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 commented 3 years ago

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

hregis commented 3 years ago

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

daraelmin commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

https://github.com/Dolibarr/dolibarr/blob/6fe5c2f5996e2f7fe8d367cad3b8e5c98d3906d6/htdocs/main.inc.php#L429-L473

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

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

daraelmin commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

https://github.com/Dolibarr/dolibarr/blob/6fe5c2f5996e2f7fe8d367cad3b8e5c98d3906d6/htdocs/main.inc.php#L457

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

hi @hregis,

I wonder if your solution is the good one cause I've found a new commit https://github.com/Dolibarr/dolibarr/commit/4c9bf1d97ea73127f9382490dd58921798e679e1 witch advice to use NOTOKENRENEWAL when using ajax.

Cheers

AXeL-dev commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

PR is here #16124

choybe commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

The last merge https://github.com/Dolibarr/dolibarr/commit/5095e7f9278ca7ace0ccfece4ba421bdc39bbc13 should fix all cases

frederic34 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

https://github.com/Dolibarr/dolibarr/blob/6fe5c2f5996e2f7fe8d367cad3b8e5c98d3906d6/htdocs/main.inc.php#L429-L473

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 commented 1 year ago

dolibarr solution... mv to trash