User tests: Successful: Unsuccessful:
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
RTC
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:
?
|
Thanks everybody
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!
@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.
@richard67 Good point, we should only re-open 38476 as the fix was for that.
@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.
@nikosdion Assigned #38476 .
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.