WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 13 months ago

#12682 assigned enhancement

Multiple password reset emails can be annoying

Reported by: SergeyBiryukov Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.9.2
Component: Users Keywords: has-patch
Focuses: Cc:

Description (last modified by SergeyBiryukov)

There's a security flaw mentioned in #10006: an attacker can bother users with password reset emails.

The problem was reported on support forums by a user receiving hundreds of these emails. He proposed to introduce some kind of a timeout for password reset requests. Is it possible?

Attachments (4)

patch.diff (2.7 KB) - added by VarunAgw 14 months ago.
Patch file for the feature
wp-login.php.diff (2.0 KB) - added by VarunAgw 14 months ago.
Patch Update #1 - Ignore earlier
wp-login.php.2.diff (2.0 KB) - added by VarunAgw 14 months ago.
Patch Update #2
wp-login.php.3.diff (2.1 KB) - added by VarunAgw 14 months ago.
Patch Update #3

Download all attachments as: .zip

Change History (22)

comment:1 @nacin5 years ago

  • Milestone Unassigned deleted
  • Resolution set to duplicate
  • Status changed from new to closed

comment:2 @dd325 years ago

nacin: Did you close this as a duplicate of #10006 or another?

comment:3 @nacin5 years ago

  • Milestone set to Unassigned
  • Resolution duplicate deleted
  • Status changed from closed to reopened

Yeah, but on second read, looks like it's related but different issues. No problem, re-opening.

comment:4 @wojtek.szkutnik5 years ago

  • Keywords gsoc added

comment:5 @dd324 years ago

  • Component changed from Security to Users
  • Keywords gsoc removed
  • Milestone changed from Awaiting Review to Future Release
  • Owner changed from ryan to dd32
  • Status changed from reopened to accepted
  • Type changed from defect (bug) to feature request

Sounds like a worthy addition to me.

My thought is:

  • Include a user meta array of password reset times
  • Before sending a reset request,
    • check the timestamps, any older than a day can get dumped
    • If count > 5? then don't send email.
  • Upon password change, clear the reset time meta.

This does sound like a problem which would only affect a small portion of the WordPress userbase, but it's an annoying effect for those who run higher profile sites I'd assume.

comment:6 @VarunAgw14 months ago

I am working on it and will upload the patch soon

comment:7 @ircbot14 months ago

This ticket was mentioned in IRC in #wordpress-dev by VarunAgw. View the logs.

@VarunAgw14 months ago

Patch file for the feature

@VarunAgw14 months ago

Patch Update #1 - Ignore earlier

@VarunAgw14 months ago

Patch Update #2

comment:8 @VarunAgw14 months ago

  • Keywords has-patch added

comment:9 @SergeyBiryukov14 months ago

  • Description modified (diff)
  • Milestone changed from Future Release to 3.9
  • Summary changed from Multiple password reset messages to Multiple password reset emails can be annoying

comment:10 @SergeyBiryukov14 months ago

We can use DAY_IN_SECONDS constant instead of 86400 (see #20987).

comment:11 @SergeyBiryukov14 months ago

  • Type changed from feature request to enhancement

comment:12 @dd3214 months ago

  • Owner dd32 deleted
  • Status changed from accepted to assigned

@VarunAgw14 months ago

Patch Update #3

comment:13 @VarunAgw14 months ago

Updated patch.

comment:14 @ircbot14 months ago

This ticket was mentioned in IRC in #wordpress-dev by VarunAgw. View the logs.

comment:15 @nikolov.tmw13 months ago

Maybe introduce a filter for the amount of requests that are allowed? So instead of :

$user_pass_requests['count'] >= 5

have

$user_pass_requests['count'] >= intval( apply_filters( 'max_resetpass_requests', 5, $user_data ) )

The same can also apply for the timeframe. This way for instance plugins can say - allow only 1 reset password request every 1 week. It would also allow(since we're passing $user_data as a second argument in the filter) specific users/user roles to have different settings(so maybe it would be less attempts for admins and more attempts for subscribers).

comment:16 @nacin13 months ago

I would suggest something a bit less harsh. I've totally done three or four password requests for a service before I realize where the heck the email is going. (Now imagine an attacker could fill up the quota.) What's the appropriate balance between two kinds of annoyances? Something to think about (and research). If someone provides an email address, is it more lenient than a username, which is public?

Implementation-wise, I think this ideally hooks in on allow_password_reset and does all of the logic there — it either updates metadata with a new timestamp or returns WP_Error if requests are being made too quickly.

comment:17 @saas13 months ago

I think there is nothing to do with core regarding this issue, core already provides several actions and filters to handle such situations, like @nacin mentioned allow_password_reset which allows you to do your own logic.

Also you would utilize retrieve_password and kill the request if you find it should not be allowed in first place, you could also use lostpassword_post but I think its too early, but its still there if you would like to use it.

I personally would like to see it as separate plugin for handling such situation, I already use "Login Limit Attempts" plugin for reducing login attempts, and its been working quite well, I have it set to maximum limits (like 12 retries and small time for lockouts), as @nacin mentioned it happens a lot to normal users as well, so its not fair to limit it to 5 or such numbers.

comment:18 @nacin13 months ago

  • Milestone changed from 3.9 to Future Release

Pushing this to "future release" for further consideration. I don't see this as making it into 3.9 at this time.

Note: See TracTickets for help on using tickets.