Page MenuHomePhabricator

PRU: Display same informational message for all password resets & fix Manual:$wgRateLimits inconsistency [high priority; small]
Closed, ResolvedPublicSecurity

Description

As a password reset user, I want the same message to appear after submitting data on Special:PasswordReset, so that information particular to my account does not get revealed to bad actors.

Background: Normally, if a user has hit their 'mailpassword' limit, when they submit the Special:PasswordReset form they are shown the "Action throttled" message. However, if the user also has their Enhanced Password Reset preference enabled and submit only their username, they are instead shown the normal password reset message (as implemented in T238961). This reveals that they have the Enhanced Password Reset preference enabled, which is arguably a disclosure of private information. For example, for user Steve (EPR enabled) and Adam (EPR disabled), submitting different parameters in Special:PasswordReset:

UsernameEmailOutcome
SteveNormal password reset message
Stevesteve@local.wmftest.net"Action throttled"
Adam"Action throttled"
Adamadam@local.wmftest.net"Action throttled"

Steps to reproduce problem:

If "Eve" has EPR enabled:

  1. Keep submitting Special:PasswordReset until I hit the $wgRateLimit for my user/IP (for most/all wikis, this is 5 attempts by the same IP within 1 hour)
  2. Submit Special:PasswordReset with Username=Eve and a blank Email

Expected behavior: "Action throttled"
Observed behavior: Normal password reset message.
Environment -- Wiki(s): MediaWiki 1.35.0-alpha (9b125a0)

Acceptance Criteria:

  • Display the same message to all users, regardless of whether they have hit their Manual:$wgRateLimits, after they have submitted any data on Special:PasswordReset
    • Redirect all users to the password informational page (see screenshot example below)
    • Update the second paragraph (see red outline in screenshot example) as follows:
      • If the information submitted is valid, a password reset email will be sent. If you haven't received an email, we recommend that you visit the reset password help page or try again later. You can only request a limited number of password resets within a short period of time. Only one password reset email will be sent per valid account every 24 hours in order to prevent abuse.
        • Be sure to follow specifications (as seen above) for bold text
        • Maintain the link behavior, as found in the previous paragraph. This means that the link to the Mediawiki help page (in the current password reset informational page) should also be present in the updated paragraph.

Visual Example (see red outline for paragraph text we should update):

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 8 2020, 4:01 PM
Reedy set Security to Software security bug.Apr 8 2020, 4:22 PM
Reedy added projects: Security, Security-Team.
Reedy changed the visibility from "Public (No Login Required)" to "Custom Policy".
Reedy changed the subtype of this task from "Bug Report" to "Security Issue".
Reedy added a subscriber: Reedy.
dom_walden added subscribers: MusikAnimal, Mooeypoo.
dom_walden added a subscriber: HMonroy.
ifried updated the task description. (Show Details)
ifried renamed this task from Throttle can reveal user's Enhanced Password Reset preferences to PRU: Throttle can reveal user's Enhanced Password Reset preferences.Apr 9 2020, 4:41 PM

@dom_walden What is the content of the "Action throttled" message?

@dom_walden What is the content of the "Action throttled" message?

"As an anti-abuse measure, you are limited from performing this action too many times in a short space of time, and you have exceeded this limit. Please try again in a few minutes."

ifried renamed this task from PRU: Throttle can reveal user's Enhanced Password Reset preferences to PRU: Throttle can reveal user's Enhanced Password Reset preferences [small].Apr 9 2020, 5:22 PM
ifried updated the task description. (Show Details)
ifried moved this task from To Be Estimated/Discussed to Estimated on the Community-Tech board.
ifried added a comment.EditedApr 10 2020, 1:01 AM

@Reedy We are thinking of resolving this issue by redirecting all users to the same informational display after they submit information (whether or not there is a 'mailpassword' limit issue). The page would display information about the mail password limit. Here is a current screenshot example of the page (without the new text that we plan to add).

The other action is to show the action throttled message to all users. Does either solution work/do you have a preference?

@Prtksxna Do you recommend we show the action throttled message to all users (which is probably easier, since no additional translations would be required) or redirect users to the next screen (since follows the general principle that all users be redirected to next screen after submitting info)? And, if we do redirect users to the next screen, we'll need to think of new text. Here is an idea: "Furthermore, you can only request a limited number of password resets within a short period of time. If you didn't receive an email, wait a few minutes and then try again." What do you think of the proposed text? Thanks!

@dom_walden I'm unable to reproduce the issue when I test. I keep on seeing the informational screen in all cases, whether the account has Enhanced Password Reset enabled. Any tips on how to reproduce this? Thanks!

@dom_walden I'm unable to reproduce the issue when I test. I keep on seeing the informational screen in all cases, whether the account has Enhanced Password Reset enabled. Any tips on how to reproduce this? Thanks!

@ifried Try it 5 times and see if you are able to get the throttle message. It's a setting that needs to be reached per wiki.

ifried added a comment.EditedApr 10 2020, 8:31 PM

@dom_walden I tried over 5 times... maybe I'm missing something.

ifried added a comment.EditedApr 10 2020, 8:39 PM

@dom_walden: Update -- reproduced it! The issue was probably that I was logged in. I was originally testing directly via the URL link for Special:PasswordReset, while logged in. In that case, when I tried repeated attempts, it was the same behavior for any username (whether or not PRU was enabled). However, when I logged out, I saw the difference. Thanks.

@dom_walden: Update -- reproduced it! The issue was probably that I was logged in. I was originally testing directly via the URL link for Special:PasswordReset, while logged in. In that case, when I tried repeated attempts, it was the same behavior for any username (whether or not PRU was enabled). However, when I logged out, I saw the difference. Thanks.

Oh, interesting. If the user you were logged in as were an admin, it might not be affected by throttles.

sbassett moved this task from Incoming to Watching on the Security-Team board.Apr 13 2020, 3:09 PM

@Prtksxna Do you recommend we show the action throttled message to all users (which is probably easier, since no additional translations would be required) or redirect users to the next screen (since follows the general principle that all users be redirected to next screen after submitting info)? And, if we do redirect users to the next screen, we'll need to think of new text. Here is an idea: "Furthermore, you can only request a limited number of password resets within a short period of time. If you didn't receive an email, wait a few minutes and then try again." What do you think of the proposed text? Thanks!

Yep, I think it makes sense to just show it to all users. The text you've drafted looks good. If we're able to find out, and if its consistent across wikis we could update "limited number of password resets" and "a short period of time" to actual figures. This might help good faith users out if they're confused.

ifried updated the task description. (Show Details)Apr 14 2020, 2:45 PM
ifried updated the task description. (Show Details)Apr 14 2020, 2:50 PM
ifried updated the task description. (Show Details)
ifried updated the task description. (Show Details)Apr 14 2020, 2:54 PM
ifried updated the task description. (Show Details)
ifried updated the task description. (Show Details)
ifried updated the task description. (Show Details)Apr 14 2020, 5:15 PM

@sbassett & @Reedy Are you okay with how we are planning to fix this issue (see acceptance criteria above)? We plan to have one unified message displayed to all users, which lets them know about various reasons why a password reset email may not have been sent. Thanks!

ifried renamed this task from PRU: Throttle can reveal user's Enhanced Password Reset preferences [small] to PRU: Throttle can reveal user's Enhanced Password Reset preferences [high priority; small].Apr 14 2020, 11:40 PM
ifried moved this task from Estimated to Kanban-2019-20-Q4 on the Community-Tech board.

We plan to have one unified message displayed to all users, which lets them know about various reasons why a password reset email may not have been sent.

Sounds good to me. Feel free to add @Reedy and I as reviewers when the patch is ready on-task or in gerrit.

Thank you so much, @sbassett! I'll let the team know.

ifried updated the task description. (Show Details)Apr 15 2020, 4:02 PM
ifried updated the task description. (Show Details)Apr 15 2020, 4:05 PM
ifried updated the task description. (Show Details)
ifried renamed this task from PRU: Throttle can reveal user's Enhanced Password Reset preferences [high priority; small] to PRU: Display same informational message for all password reset & fix Manual:$wgRateLimits inconsistency[high priority; small].Apr 15 2020, 4:22 PM
ifried renamed this task from PRU: Display same informational message for all password reset & fix Manual:$wgRateLimits inconsistency[high priority; small] to PRU: Display same informational message for all password resets & fix Manual:$wgRateLimits inconsistency [high priority; small].
HMonroy claimed this task.Apr 15 2020, 5:29 PM
HMonroy moved this task from Ready to In Development on the Community-Tech (Kanban-2019-20-Q4) board.
HMonroy added a comment.EditedApr 15 2020, 8:46 PM

The specifications in this ticket have been implemented. Please note that there was a loophole in the code. The logic that checks for the $wgRateLimit has been moved above the Enhanced Password Reset check so it is possible to keep the Action throttle message if we wanted to. By moving the mailpasswordlogic, we avoid the possibility of disclosing any private information if we decided to keep and trigger the Action throttle message. The users would see the throttle message whether they have enabled PRU or not.

dom_walden added a comment.EditedApr 16 2020, 1:32 PM

@HMonroy @ifried Ugh, I've found another set of conditions where this also happens:

If I submit the Special:PasswordReset form with a username which has PRU enabled and an invalid email (e.g. "email|address"), the form returns the message "Invalid email address".

For a username with PRU disabled, the form submits and returns the usual message ("If the information submitted is valid...").

Same or separate ticket?

You might have to programmatically submit the form (e.g. via curl), as most/all browsers implement their own form of email validation which usual prevents invalid email addresses being submitted.

For example, curl -X 'POST' 'http://dev.wiki.local.wmftest.net:8080/w/index.php?title=Special:PasswordReset' --data "wpUsername=Eve&wpEmail=foo|bar&title=Special%3APasswordReset&redirectparams="

@dom_walden Thanks for catching this!

@HMonroy Can this be fixed in this ticket, or no? We want the behavior to be that all users are redirected to the next informational screen. Thanks!

This comment was removed by HMonroy.
HMonroy added a comment.EditedApr 16 2020, 4:12 PM

@dom_walden Thanks! I see where the problem is.

@ifried The user gets the "Invalid email address" error when they submit emails like: "foo", "foo@wiki", "wiki.com". I just want to verify that we are eliminating the "Invalid email address" altogether and showing the general message instead.

I'll create a followup patch, no need to create a separate ticket.

@HMonroy Yes, let's still direct users to the next screen. They will see the information that they input in the informational screen, so they should be able to ascertain if the email address is correct. I'll also update the documentation to cover cases associated with incomplete email addresses.

This comment was removed by ifried.

When throttled, the user no longer sees some (perhaps all?) of the validation messages they would see when not throttled. These include:

  • When entering an invalid username (e.g. "user|name")

Not throttled:


Throttled:

  • When entering a blank username and email

Not throttled:


Throttled:

I don't know of any others, but there might be.

@ifried Is this ok?

Thanks for catching this, @dom_walden! Yes, I think this is okay. Here's why:

  • This merely exposes throttling (which was previously exposed).
  • We are not exposing any information related to user accounts or preferences.

For these reasons, I don't think that this is a blocker for us to enable the 'Enhanced Password Reset' on Wikipedia . In addition, I don't think this needs to be immediately changed (though it can be in the future). If the security team wishes to further standardize the process, they can certainly do so. But I think we should be okay with the current state of the work, which has improved security overall. Thanks!

ifried added a comment.May 4 2020, 5:45 PM

Note: This is now on production. I have tested it on production, and it looks good. But I'll wait for Dom to officially move this to product sign-off before I close it. Thanks!

@ifried - We should also make this task public once it's completely resolved. I suppose this would've been iffy to hold for the next security release (T248535), since it's likely not a major issue, but that doesn't really matter since the patch sets for master are public:

We'll probably want to see about backporting those to supported release branches, if feasible.

I can no longer reproduce the bugs in the description and T249730#6062487.

We no longer appear to show the "Action throttled" in any circumstances (that I have seen so far).

Similarly to T246844#5991422, I submitted Special:PasswordReset with a combination of user + email on https://wikidata.beta.wmflabs.org (which has PRU).

Details below. The only issues I can see are those already described in T249730#6089025.

Setup:

PRU enabledEmail set24hr limit hitUsernameEmail
TrueTrueTrueDrwpbdrwpb@dwalden.co.uk
TrueTrueFalseDrwpb1drwpb1@dwalden.co.uk
FalseTrueTrueDrwprdrwpr@dwalden.co.uk
FalseTrueFalseDrwbbvdrwbbv@dwalden.co.uk
FalseFalseFalseDrwbbv1

Results:

UsernameEmailThrottleValidation/Error MessageEmail
unthrottled"Neither a username nor an email address was supplied"
drwbbv@dwalden.co.ukunthrottled#2Drwbbv
drwpb1@dwalden.co.ukunthrottled#2
drwpb@dwalden.co.ukunthrottled#2
drwpr@dwalden.co.ukunthrottled#2
emailPIPEaddressunthrottled#2
sdfjklsdj@sdjfklsd.comunthrottled#2
userPIPEnameunthrottled"You have not specified a valid username."
userPIPEnamedrwbbv@dwalden.co.ukunthrottled"You have not specified a valid username."
userPIPEnamedrwpb1@dwalden.co.ukunthrottled"You have not specified a valid username."
userPIPEnamedrwpb@dwalden.co.ukunthrottled"You have not specified a valid username."
userPIPEnamedrwpr@dwalden.co.ukunthrottled"You have not specified a valid username."
userPIPEnameemailPIPEaddressunthrottled"You have not specified a valid username."
userPIPEnamesdfjklsdj@sdjfklsd.comunthrottled"You have not specified a valid username."
sjfklsdjfklunthrottled#1
sjfklsdjfkldrwbbv@dwalden.co.ukunthrottled#3
sjfklsdjfkldrwpb1@dwalden.co.ukunthrottled#3
sjfklsdjfkldrwpb@dwalden.co.ukunthrottled#3
sjfklsdjfkldrwpr@dwalden.co.ukunthrottled#3
sjfklsdjfklemailPIPEaddressunthrottled#3
sjfklsdjfklsdfjklsdj@sdjfklsd.comunthrottled#3
Drwpbunthrottled#1
Drwpbdrwbbv@dwalden.co.ukunthrottled#3
Drwpbdrwpb1@dwalden.co.ukunthrottled#3
Drwpbdrwpb@dwalden.co.ukunthrottled#3
Drwpbdrwpr@dwalden.co.ukunthrottled#3
DrwpbemailPIPEaddressunthrottled#3
Drwpbsdfjklsdj@sdjfklsd.comunthrottled#3
Drwpb1unthrottled#1
Drwpb1drwbbv@dwalden.co.ukunthrottled#3
Drwpb1drwpb1@dwalden.co.ukunthrottled#3Drwpb1
Drwpb1drwpb@dwalden.co.ukunthrottled#3
Drwpb1drwpr@dwalden.co.ukunthrottled#3
Drwpb1emailPIPEaddressunthrottled#3
Drwpb1sdfjklsdj@sdjfklsd.comunthrottled#3
Drwprunthrottled#1
Drwprdrwbbv@dwalden.co.ukunthrottled#3
Drwprdrwpb1@dwalden.co.ukunthrottled#3
Drwprdrwpb@dwalden.co.ukunthrottled#3
Drwprdrwpr@dwalden.co.ukunthrottled#3
DrwpremailPIPEaddressunthrottled#3
Drwprsdfjklsdj@sdjfklsd.comunthrottled#3
Drwbbvunthrottled#1Drwbbv
Drwbbvdrwbbv@dwalden.co.ukunthrottled#3Drwbbv
Drwbbvdrwpb1@dwalden.co.ukunthrottled#3Drwbbv
Drwbbvdrwpb@dwalden.co.ukunthrottled#3Drwbbv
Drwbbvdrwpr@dwalden.co.ukunthrottled#3Drwbbv
DrwbbvemailPIPEaddressunthrottled#3Drwbbv
Drwbbvsdfjklsdj@sdjfklsd.comunthrottled#3Drwbbv
Drwbbv1unthrottled#1
Drwbbv1drwbbv@dwalden.co.ukunthrottled#3
Drwbbv1drwpb1@dwalden.co.ukunthrottled#3
Drwbbv1drwpb@dwalden.co.ukunthrottled#3
Drwbbv1drwpr@dwalden.co.ukunthrottled#3
Drwbbv1emailPIPEaddressunthrottled#3
Drwbbv1sdfjklsdj@sdjfklsd.comunthrottled#3
throttled#0
emailPIPEaddressthrottled#2
sdfjklsdj@sdjfklsd.comthrottled#2
drwpb@dwalden.co.ukthrottled#2
drwpb1@dwalden.co.ukthrottled#2
drwpr@dwalden.co.ukthrottled#2
drwbbv@dwalden.co.ukthrottled#2
userPIPEnamethrottled#1
userPIPEnameemailPIPEaddressthrottled#3
userPIPEnamesdfjklsdj@sdjfklsd.comthrottled#3
userPIPEnamedrwpb@dwalden.co.ukthrottled#3
userPIPEnamedrwpb1@dwalden.co.ukthrottled#3
userPIPEnamedrwpr@dwalden.co.ukthrottled#3
userPIPEnamedrwbbv@dwalden.co.ukthrottled#3
sjfklsdjfklthrottled#1
sjfklsdjfklemailPIPEaddressthrottled#3
sjfklsdjfklsdfjklsdj@sdjfklsd.comthrottled#3
sjfklsdjfkldrwpb@dwalden.co.ukthrottled#3
sjfklsdjfkldrwpb1@dwalden.co.ukthrottled#3
sjfklsdjfkldrwpr@dwalden.co.ukthrottled#3
sjfklsdjfkldrwbbv@dwalden.co.ukthrottled#3
Drwpbthrottled#1
DrwpbemailPIPEaddressthrottled#3
Drwpbsdfjklsdj@sdjfklsd.comthrottled#3
Drwpbdrwpb@dwalden.co.ukthrottled#3
Drwpbdrwpb1@dwalden.co.ukthrottled#3
Drwpbdrwpr@dwalden.co.ukthrottled#3
Drwpbdrwbbv@dwalden.co.ukthrottled#3
Drwpb1throttled#1
Drwpb1emailPIPEaddressthrottled#3
Drwpb1sdfjklsdj@sdjfklsd.comthrottled#3
Drwpb1drwpb@dwalden.co.ukthrottled#3
Drwpb1drwpb1@dwalden.co.ukthrottled#3
Drwpb1drwpr@dwalden.co.ukthrottled#3
Drwpb1drwbbv@dwalden.co.ukthrottled#3
Drwprthrottled#1
DrwpremailPIPEaddressthrottled#3
Drwprsdfjklsdj@sdjfklsd.comthrottled#3
Drwprdrwpb@dwalden.co.ukthrottled#3
Drwprdrwpb1@dwalden.co.ukthrottled#3
Drwprdrwpr@dwalden.co.ukthrottled#3
Drwprdrwbbv@dwalden.co.ukthrottled#3
Drwbbvthrottled#1
DrwbbvemailPIPEaddressthrottled#3
Drwbbvsdfjklsdj@sdjfklsd.comthrottled#3
Drwbbvdrwpb@dwalden.co.ukthrottled#3
Drwbbvdrwpb1@dwalden.co.ukthrottled#3
Drwbbvdrwpr@dwalden.co.ukthrottled#3
Drwbbvdrwbbv@dwalden.co.ukthrottled#3
Drwbbv1throttled#1
Drwbbv1emailPIPEaddressthrottled#3
Drwbbv1sdfjklsdj@sdjfklsd.comthrottled#3
Drwbbv1drwpb@dwalden.co.ukthrottled#3
Drwbbv1drwpb1@dwalden.co.ukthrottled#3
Drwbbv1drwpr@dwalden.co.ukthrottled#3
Drwbbv1drwbbv@dwalden.co.ukthrottled#3

#0 = "If the information submitted is valid, a password reset email will be sent..." with neither Username nor Email displayed (as shown in T249730#6089025).
#1 = "If the information submitted is valid, a password reset email will be sent..." with Username displayed.
#2 = "If the information submitted is valid, a password reset email will be sent..." with Email displayed.
#3 = "If the information submitted is valid, a password reset email will be sent..." with Username and Email displayed.
userPIPEname = "user|name", but I cannot put pipes into tables in Phab

ifried added a comment.May 5 2020, 6:34 PM

Thank you, @dom_walden!

@HMonroy: This ticket has now been moved to product sign-off. So, we can proceed with enabling Enhanced Password Reset on Wikipedia (as per T245791) on Wednesday. Thanks!

ifried closed this task as Resolved.May 6 2020, 4:47 PM

@ifried @dom_walden @HMonroy - I'd like to make this task public especially since the code changes were done publicly in gerrit anyways. I'm not seeing any PII or overly-sensitive data on the task unless, @dom_walden, you're not comfortable with some of the test data you've posted. Let me know, thanks.

@ifried @dom_walden @HMonroy - I'd like to make this task public especially since the code changes were done publicly in gerrit anyways. I'm not seeing any PII or overly-sensitive data on the task unless, @dom_walden, you're not comfortable with some of the test data you've posted. Let me know, thanks.

I am fine with my data being public.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".May 21 2020, 4:58 PM

Change 597829 abandoned by SBassett:
[SECURITY] Password Reset Updates

Reason:
DatabaseBlock only available from 1.34 . Will attempt backport to that branch.

https://gerrit.wikimedia.org/r/597829

Change 601437 had a related patch set uploaded (by SBassett; owner: HMonroy):
[mediawiki/core@REL1_34] [SECURITY] Password Reset Updates

https://gerrit.wikimedia.org/r/601437

Change 601437 merged by jenkins-bot:
[mediawiki/core@REL1_34] [SECURITY] Password Reset Updates

https://gerrit.wikimedia.org/r/601437

sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.