[4.x] - com_finder doesn't stop indexing on CSRF token mismatch

[SniperSister]

Summary of Changes

Added the required "return" statements after sending the error message to prevent the process from continuing.

Testing Instructions

1. run the com_finder indexer from the backend and make sure it works
2. edit the line
   <input id="finder-indexer-token" type="hidden" name="<?php echo Factory::getSession()->getFormToken(); ?>" value="1"> to
   <input id="finder-indexer-token" type="hidden" name="123" value="1">
   in administrator/components/com_finder/tmpl/indexer/default.php and hit the "reindex" button again - an error message is shown.

Actual result BEFORE applying this Pull Request

The error message is test case two isn't shown because the returned response is invalid

Expected result AFTER applying this Pull Request

Error is shown

[SniperSister]

FYI @joomla/security

[zero-24]

LGTM 👍

[HLeithner]

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

[zero-24]

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.

[Fedik]

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

[ceford]

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?

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29979.}

[ceford]

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

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29979.}

[SniperSister]

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?

[Quy]

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");

[ceford]

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

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29979.}

[SniperSister]

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

[wilsonge]

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

[particthistle]

I have tested this item 🔴 unsuccessfully on da7a77d

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

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29979.}

[particthistle]

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:

[wilsonge]

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

[wilsonge]

My screen

[particthistle]

I have tested this item ✅ successfully on 797aa4d

Changing token after applying PR results in error message.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29979.}

[ceford]

I have tested this item ✅ successfully on 797aa4d

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29979.}

[wilsonge]

Thanks!