FYI @joomla/security

Any idea why this is not at the beginning of the functions?

The response after this PR is

{"token":"3c17b81ea84946fcec0eed888d382ecb","error":true,"header":"An Error Has Occurred","message":"The security token did not match. The request was aborted to prevent any security breach. Please try again.","buffer":""}

thus revealing the correct token and allowing the hacker to make the same call again from their remote location, including that correct token, without additional work - you are literally handing the CSRF attacker what he needs to be able to be successful.

These Ajax calls are a destructive call - the start function resets the index state.

Therefore at a minimum it should only respond to a POST and not a GET. It currently responds to a GET.

As its also a destructive call (or a call that doesnt only get data, but manipulates it) it should be protected by a CSRF token. Else a CSRF compromise (like you hosting an image from my domain name) can allow me to trigger the destructive action of resetting your index state !

Even with your changes, The token check should be the very first thing in the methods too. Else, with logging enabled, the hacked request can still write a line to your logger, an action I should not be able to do via a CSRF vulnerability. Using static:sendResponse also writes to the log files.

I also disagree that this should have gone public, its an active CRSF vulnerability, but too late now.

Also @joomla/security doesnt appear to be a valid team on GitHub any longer.

Also @joomla/security doesnt appear to be a valid team on GitHub any longer.

IIRC github limts mention of teams to member of the org. I can confirm that this is still a valid team.

Any idea why this is not at the beginning of the functions?

maybe for errors logging, that is at the beginning of the function :)
if so, then this error also has to be logged

I removed said line and clicked the Index button in the Smart Search: Indexed Content page. The Modal dialog appeared and hung - am I supposed to look somewhere else for the error?

I should have said - both with and without the patch.

thus revealing the correct token and allowing the hacker to make the same call again from their remote location, including that correct token, without additional work - you are literally handing the CSRF attacker what he needs to be able to be successful.

Literally any response with a form returns that token to the active user, that's how CSRF tokens are designed. I agree that it can be removed (and did so by updating this PR) however I can't see any risk associated with this behavior.

The token check should be the very first thing in the methods too

Done

The Modal dialog appeared and hung - am I supposed to look somewhere else for the error?

Is it just a blank page? Or do you have any error message or output in there?

I have tested this item ✅ successfully on da7a77d

The modal hangs. An error message is not displayed. In the console:

Uncaught TypeError: document.getElementById(...) is null

var token = "&".concat(document.getElementById('finder-indexer-token').getAttribute('name'), "=1");

With new update to Beta4-Dev, after removing the line and applying the patch the modal hangs - so I think this is a fail!

@ceford @Quy I updated the test instructions, please retry

I suspect (don't have a way to test right now) that if there's an error thrown in a plugin that this will now longer be indexable second time around by dropping the token. Fedik's sample code shows the token in the response is being used. I don't see why we should remove it here. It's no different to submitting a frontend login form that has an invalid token

I have tested this item 🔴 unsuccessfully on da7a77d

Tested during BFH tonight as Patch Tester was showing it as a release blocker.

Failed with the error message not being displayed. The Indexer just hangs when you click on it when the token has been manually changed.

Additionally, developer tools shows the following:

@particthistle you able to test this one again? Think I've fixed that bug (now requires NPM to be run I'm afraid)

My screen

I have tested this item ✅ successfully on 797aa4d

Changing token after applying PR results in error message.

I have tested this item ✅ successfully on 797aa4d

Thanks!