? ? Pending

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
17 Aug 2022

Pull Request for Issue #38476 and #38436 .

Summary of Changes

MFA redirect should not happen on non HTML request

Testing Instructions

Please follow #38436 and #38476

1fd81f6 17 Aug 2022 avatar Fedik Typo
avatar Fedik Fedik - open - 17 Aug 2022
avatar Fedik Fedik - change - 17 Aug 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 17 Aug 2022
Category Libraries
avatar bayareajenn
bayareajenn - comment - 17 Aug 2022

I have tested this item successfully on 1fd81f6


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

avatar bayareajenn bayareajenn - test_item - 17 Aug 2022 - Tested successfully
avatar viocassel
viocassel - comment - 17 Aug 2022

I have tested this item successfully on 1fd81f6


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

avatar viocassel viocassel - test_item - 17 Aug 2022 - Tested successfully
avatar richard67 richard67 - change - 17 Aug 2022
Status Pending Ready to Commit
Labels Added: ?
avatar richard67
richard67 - comment - 17 Aug 2022

RTC


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

avatar roland-d roland-d - change - 17 Aug 2022
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-08-17 18:39:56
Closed_By roland-d
Labels Added: ?
avatar roland-d roland-d - close - 17 Aug 2022
avatar roland-d roland-d - merge - 17 Aug 2022
avatar roland-d
roland-d - comment - 17 Aug 2022

Thanks everybody

avatar nikosdion
nikosdion - comment - 18 Aug 2022

From a security perspective, this is wrong.

I am an attacker. I steal someone's username and password. I log into the site. I am in the captive page. Now I can run any non-HTML context with the full privileges of the user whose credentials I stole and nobody will stop me.

First of all, this is a massive security hole for GET requests because I can retrieve any data the user has access to. If I have stolen the credentials of an Administrator or Super User it's very likely I have access to a component's JSON or CSV view which gives me access to personally identifiable information. Congratulations, your get several thousands to millions of Euros in fines thanks to GDPR.

Second, it now becomes TRIVIAL for me to nerf your site as my POST requests can proceed uninhibited... as long as I send format=bozo to the POST request. Whine whine we have the anti-CSRF token whine whine. Bollocks. The captive page contains the anti-CSRF token and any time I want to fetch a new one I just have to GET index.php with no arguments. If I've stollen the credentials of a Publisher or above I can deface your site at the very least.

I HAD ALREADY EXPLAINED ALL OF THESE IN MAY, THE FIRST TIME @Fedik HAD THIS DISASTROUS IDEA. So what did you do here? You just undid all the good that captive Multi-factor Authentication brought to Joomla.

@roland-d You need to revert it. Moreover, you and @fancyFranci ARE SUPPOSED TO ASK THE CODE OWNER, THAT'S ME, BEFORE MERGING ANY PR REGARDING THE WEBAUTHN AND THE MFA FEATURES. I am the code owner of these features for a reason!

avatar roland-d
roland-d - comment - 18 Aug 2022

@nikosdion Thank you for catching this. I have reverted this now and we can check later how it should be handled as it does show an error when users click on the button.

As for contacting the code owner, I understand that but I never thought about contacting a code owner because I was not triggered by anything. Yes, I know you contributed this code but I do not know that for every feature in Joomla. Having some kind of notification to contact the code owner would be helpful. Perhaps we should add you to the codeowners file if that helps, I do not know.

avatar richard67
richard67 - comment - 18 Aug 2022

@roland-d Shouldn't we re-open the 2 issues #38476 and #38436 as long as we don't have the solution?

avatar roland-d
roland-d - comment - 18 Aug 2022

@richard67 Good point, we should only re-open 38476 as the fix was for that.

avatar nikosdion
nikosdion - comment - 18 Aug 2022

@roland-d I am always going through the commits before a new release because I am weird like that :p

Regarding code ownership, I think there are two problems. One is GitHub and the other is the institutional knowledge gap which is a chronic Joomla pain.

I had been added in the .github/CODEOWNERS file for the WebAuthn plugin and the MFA. However, since I am a lowly Collaborator and not a full Member of the repo, GitHub would NOT auto-assign me as a reviewer for PRs touching my code. Therefore this change never made it to the repo. Even if you add me now, it will be for naught (and I think I was told it was creating problems, dunno?)

So, I have a question. Can I get into the CMS maintenance team? At this point I am pretty much doing the work anyway so if it means I can actually get to be notified for PRs touching security-sensitive code I have contributed so much the better. I'd prefer if there was a way to do that without having to carry a badge, albeit GitHub won't play ball. Well, in for a dime, in for a dollar.

@richard67 I had already said how we can address that problem:

Another way to do it — best practice for 3PDs actually — is for the plugin to check if we're in a captive page and not execute. I am leaving this note not as something applicable to this PR but as something that a 3PD might stumble on. The rabbit hole will lead them to this comment here and they will figure out what to do next ;)

If you assign #38436 and #38476 to me I can make it happen, tomorrow, after a morning swimming session.

avatar richard67
richard67 - comment - 18 Aug 2022

@nikosdion Assigned #38476 .

Add a Comment

Login with GitHub to post a comment