? Pending

User tests: Successful: Unsuccessful:

avatar SniperSister
SniperSister
25 Oct 2020

Summary of Changes

Enforce the usage of POST requests for administrator login requests – doesn't affect core CMS as it's doing POST logins by default anyway.

Testing Instructions

Apply patch, login.

Actual result BEFORE applying this Pull Request

Login works

Expected result AFTER applying this Pull Request

Login works

avatar SniperSister SniperSister - open - 25 Oct 2020
avatar SniperSister SniperSister - change - 25 Oct 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 25 Oct 2020
Category Administration com_login
avatar PhilETaylor
PhilETaylor - comment - 25 Oct 2020

This fixes the reported security issue. Im not sure why we are now resolving Joomla 4 security issues in public repeatedly now though.

'post' is not needed. That is the default for the first param of checkToken

Did you take a wider look, or specific to the report? What about the other 13 uses of checkToken('request') - have you ensured those are also not introducing a security issue? (hint, there is another security issue

Should a GET have the ability to set the default "home" property for a list of items by GET request, when it should only be able to be set by POST (@since 1.6)

Surely the ones using 'request' should be more specifically a "get" or a "post" (which is the default when no method set)? a method that can be called by GET and POST is probably badly coded I guess.

$this->checkToken('request'); is also used in the "Method to confirm the password request." - is that ok in context?

avatar zero-24
zero-24 - comment - 25 Oct 2020

This fixes the reported security issue. Im not sure why we are now resolving Joomla 4 security issues in public repeatedly now though.

Joomla 4 is not released stable yet and so a pre release. As long as the reported issue is an issue only within 4.x only or as in this case only will be fixed within 4.x it got an PR in the public tracker.

Reporting it to Security@ helps to check that distinction upfront for sure.

Edit: just to make that clear the above staremant is true until 4.x is in stable state. ;-)

avatar ceford ceford - test_item - 27 Oct 2020 - Tested successfully
avatar ceford
ceford - comment - 27 Oct 2020

I have tested this item successfully on ddd8f5c


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31236.

avatar gostn gostn - test_item - 27 Oct 2020 - Tested successfully
avatar gostn
gostn - comment - 27 Oct 2020

I have tested this item successfully on ddd8f5c


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31236.

avatar wilsonge wilsonge - change - 1 Nov 2020
Labels Added: ? ?
avatar wilsonge wilsonge - change - 1 Nov 2020
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-11-01 23:02:19
Closed_By wilsonge
Labels Removed: ?
avatar wilsonge wilsonge - close - 1 Nov 2020
avatar wilsonge wilsonge - merge - 1 Nov 2020
avatar wilsonge
wilsonge - comment - 1 Nov 2020

Thanks!

avatar PhilETaylor
PhilETaylor - comment - 15 May 2022

Did you take a wider look, or specific to the report? What about the other 13 uses of checkToken('request') - have you ensured those are also not introducing a security issue? (hint, there is another security issue

Oh well, you can only lead a horse to water, you cannot make it drink... Im done.

Add a Comment

Login with GitHub to post a comment