Logout race condition

[josestefan]

Background information

IMPORTANT: If you choose to ignore this issue report template, your issue will be closed as we cannot help without the requested information.

Please make sure you tick (add an x between the square brackets with no spaces) the following check boxes:

• Reporting an issue of an unmodified OSPOS installation
• Checked open and closed issues and no similar issue was already reported (please make sure you searched!)
• Read README, WHATS_NEW, INSTALL.md and UPGRADE
• Read the FAQ for any known install and/or upgrade gotchas (in specific PHP extensions installed)
• Read the wiki
• Executed any database upgrade scripts if an upgrade pre 3.0.0 (e.g. database/2.4_to_3.0.sql)
• Aware the installation code that GitHub master is for developers only and therefore not complete nor stable.

Installation information

• OSPOS version is: 3.3.8
• OSPOS git commit hash is: ffe492
• PHP version is: 7.4.3
• MySQL or MariaDB version is: 10.6.7-MariaDB-2ubuntu1.1 Ubuntu 22.04
• OS and version is: Ubuntu 22.04.1 LTS
• WebServer is: Apache/2.4.52 (Ubuntu)
• Selected language is: English
• (If applicable) Docker installation:
• (If applicable) Installation package for the LAMP/LEMP stack is:

Issue / Bug / Question / New Feature

I'm using Firefox (105.0.3 - 64bit) as my web browser, and noticed the logout button doesn't always work. Using the Firefox developer console Network tab, I get the following response:
NS_BINDING_ABORTED
I also noticed the page using (xhr) aka AJAX

But spamming the logout button, eventually causes it to work. There seems to be a race condition at play. Googling the error, one of the possibilities is the web browser aborted the operation. Seems the "#" part of the A tag is causing the underlying ajax to abort. Unless it happens to run fast enough (Race condition) that the logout actually happens.

In most cases, I can click logout many times, and afterwards refresh the page, and I'm still logged in. I have to be really lucky with this race condition to happen. I guess the latency between the user and server could play a big role in this, as I'm using a VPS server and not a server on my LAN.

Looking at the HTML code of the page as generated, there seem to be no call to "event.preventDefault()" which would stop the browser from performing the #. So I did a test using the tools on Firefox's dev console.

1. I edited the source of the page live, Using Inspect and adding the "; event.preventDefault()"
2. I clicked the logout link.
3. I checked the output of the network tab and noticed it went thru.
4. But noticed the page does THEN depend on the "#" to actually navigate out, and it's now missing.
5. If I refresh at this point, it will go back to the login page, indicating I'm in fact logout but it was never presented to me.
   OR alternatively. I re-edit the link to put it how it was, click it again, and that also takes me back to the login page.

Therefore I suggest:

• Integrating event.preventDefault() as part of the onclick
• Navigating away as part of the AJAX triggered response, using ajax trigger events.

Alternatively, It seems to me using AJAX to logout is rather unnecessary. You'd need to "navigate" to the login page anyway. So it would be much easier to navigate to the logout page, which would send the browser a 302 header redirecting to the login page.

[josestefan]

Did another quick test,

Manually navigated to the post url using my browser's address bar:
Result I get properly logged out and sent to the login page.

Therefore using a regular link would suffice, instead of post and ajax.

[jekkos]

Hi, we recently changed the logout to POST as otherwise there would be a CSRF issue. A redirect or plain GET can be triggered by any 3rd party website which is an issue.

[jekkos]

I guess the proper way to handle it is do the redirect once the callback of the post request is triggered. Now it seems to be something in between.

[josestefan]

Thanks for the fix.

How exactly does the POST method prevent a CSRF? the underlying GET seems to still work, wouldn't that have to also be disabled/ignored?

I'm not sure what the exploit would be, but if we are worried someone can trick a user into visiting a logout link. Then yeah, that would still work as of 3.3.8

[jekkos]

Indeed you are correct, I have amended the patch to also deny GET requests now. It should be deployed on dev soon.
The exploit is a denial of service I suppose.. that's about it

[jekkos]

Fix now in master