Skip to content

Commit 5b58664

Browse files
dd32dd32
committed
Expire password reset links after 24 hours (by default). This causes existing password reset links to become invalid.
Props markjaquith, voldemortensen, johnbillion, MikeHansenMe, dd32 See #32429 git-svn-id: https://develop.svn.wordpress.org/trunk@33019 602fd350-edb4-49c9-b593-d223f7449a82
1 parent b04ea26 commit 5b58664

3 files changed

Lines changed: 158 additions & 9 deletions

File tree

‎src/wp-includes/user.php‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2458,18 +2458,42 @@ function check_password_reset_key($key, $login) {
24582458
$wp_hasher = new PasswordHash( 8, true );
24592459
}
24602460

2461-
if ( $wp_hasher->CheckPassword( $key, $row->user_activation_key ) )
2461+
/**
2462+
* Filter the expiration time of password reset keys.
2463+
*
2464+
* @since 4.3.0
2465+
*
2466+
* @param int $expiration The expiration time in seconds.
2467+
*/
2468+
$expiration_duration = apply_filters( 'password_reset_expiration', DAY_IN_SECONDS );
2469+
2470+
if ( false !== strpos( $row->user_activation_key, ':' ) ) {
2471+
list( $pass_request_time, $pass_key ) = explode( ':', $row->user_activation_key, 2 );
2472+
$expiration_time = $pass_request_time + $expiration_duration;
2473+
} else {
2474+
$pass_key = $row->user_activation_key;
2475+
$expiration_time = false;
2476+
}
2477+
2478+
$hash_is_correct = $wp_hasher->CheckPassword( $key, $pass_key );
2479+
2480+
if ( $hash_is_correct && $expiration_time && time() < $expiration_time ) {
24622481
return get_userdata( $row->ID );
2482+
} elseif ( $hash_is_correct && $expiration_time ) {
2483+
// Key has an expiration time that's passed
2484+
return new WP_Error( 'expired_key', __( 'Invalid key' ) );
2485+
}
24632486

2464-
if ( $key === $row->user_activation_key ) {
2487+
if ( hash_equals( $row->user_activation_key, $key ) || ( $hash_is_correct && ! $expiration_time ) ) {
24652488
$return = new WP_Error( 'expired_key', __( 'Invalid key' ) );
24662489
$user_id = $row->ID;
24672490

24682491
/**
24692492
* Filter the return value of check_password_reset_key() when an
2470-
* old-style key is used (plain-text key was stored in the database).
2493+
* old-style key is used.
24712494
*
2472-
* @since 3.7.0
2495+
* @since 3.7.0 Previously plain-text keys were stored in the database.
2496+
* @since 4.3.0 Previously key hashes were stored without an expiration time.
24732497
*
24742498
* @param WP_Error $return A WP_Error object denoting an expired key.
24752499
* Return a WP_User object to validate the key.

‎src/wp-login.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ function retrieve_password() {
363363
require_once ABSPATH . WPINC . '/class-phpass.php';
364364
$wp_hasher = new PasswordHash( 8, true );
365365
}
366-
$hashed = $wp_hasher->HashPassword( $key );
366+
$hashed = time() . ':' . $wp_hasher->HashPassword( $key );
367367
$wpdb->update( $wpdb->users, array( 'user_activation_key' => $hashed ), array( 'user_login' => $user_login ) );
368368

369369
$message = __('Someone requested that the password be reset for the following account:') . "\r\n\r\n";
@@ -528,10 +528,11 @@ function retrieve_password() {
528528
}
529529

530530
if ( isset( $_GET['error'] ) ) {
531-
if ( 'invalidkey' == $_GET['error'] )
532-
$errors->add( 'invalidkey', __( 'Sorry, that key does not appear to be valid.' ) );
533-
elseif ( 'expiredkey' == $_GET['error'] )
534-
$errors->add( 'expiredkey', __( 'Sorry, that key has expired. Please try again.' ) );
531+
if ( 'invalidkey' == $_GET['error'] ) {
532+
$errors->add( 'invalidkey', __( 'Your password reset link appears to be invalid. Please request a new lnk below.' ) );
533+
} elseif ( 'expiredkey' == $_GET['error'] ) {
534+
$errors->add( 'expiredkey', __( 'Your password reset link has expired. Please request a new link below.' ) );
535+
}
535536
}
536537

537538
$lostpassword_redirect = ! empty( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : '';

‎tests/phpunit/tests/auth.php‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@
66
*/
77
class Tests_Auth extends WP_UnitTestCase {
88
var $user_id;
9+
var $wp_hasher;
910

1011
function setUp() {
1112
parent::setUp();
1213
$this->user_id = $this->factory->user->create();
14+
15+
require_once ABSPATH . WPINC . '/class-phpass.php';
16+
$this->wp_hasher = new PasswordHash( 8, true );
1317
}
1418

1519
function test_auth_cookie_valid() {
@@ -159,4 +163,124 @@ function test_password_length_limit() {
159163
// Password broken by setting it to be too long.
160164
$this->assertInstanceOf( 'WP_Error', $user );
161165
}
166+
167+
/**
168+
* @ticket 32429
169+
*/
170+
function test_user_activation_key_is_checked() {
171+
global $wpdb;
172+
173+
$key = wp_generate_password( 20, false );
174+
$user = $this->factory->user->create_and_get();
175+
$wpdb->update( $wpdb->users, array(
176+
'user_activation_key' => strtotime( '-1 hour' ) . ':' . $this->wp_hasher->HashPassword( $key ),
177+
), array(
178+
'ID' => $user->ID,
179+
) );
180+
181+
// A valid key should be accepted
182+
$check = check_password_reset_key( $key, $user->user_login );
183+
$this->assertInstanceOf( 'WP_User', $check );
184+
$this->assertSame( $user->ID, $check->ID );
185+
186+
// An invalid key should be rejected
187+
$check = check_password_reset_key( 'key', $user->user_login );
188+
$this->assertInstanceOf( 'WP_Error', $check );
189+
190+
// An empty key should be rejected
191+
$check = check_password_reset_key( '', $user->user_login );
192+
$this->assertInstanceOf( 'WP_Error', $check );
193+
194+
// A truncated key should be rejected
195+
$partial = substr( $key, 0, 10 );
196+
$check = check_password_reset_key( $partial, $user->user_login );
197+
$this->assertInstanceOf( 'WP_Error', $check );
198+
}
199+
200+
/**
201+
* @ticket 32429
202+
*/
203+
function test_expired_user_activation_key_is_rejected() {
204+
global $wpdb;
205+
206+
$key = wp_generate_password( 20, false );
207+
$user = $this->factory->user->create_and_get();
208+
$wpdb->update( $wpdb->users, array(
209+
'user_activation_key' => strtotime( '-48 hours' ) . ':' . $this->wp_hasher->HashPassword( $key ),
210+
), array(
211+
'ID' => $user->ID,
212+
) );
213+
214+
// An expired but otherwise valid key should be rejected
215+
$check = check_password_reset_key( $key, $user->user_login );
216+
$this->assertInstanceOf( 'WP_Error', $check );
217+
}
218+
219+
/**
220+
* @ticket 32429
221+
*/
222+
function test_empty_user_activation_key_fails_key_check() {
223+
global $wpdb;
224+
225+
$user = $this->factory->user->create_and_get();
226+
227+
// An empty user_activation_key should not allow any key to be accepted
228+
$check = check_password_reset_key( 'key', $user->user_login );
229+
$this->assertInstanceOf( 'WP_Error', $check );
230+
231+
// An empty user_activation_key should not allow an empty key to be accepted
232+
$check = check_password_reset_key( '', $user->user_login );
233+
$this->assertInstanceOf( 'WP_Error', $check );
234+
}
235+
236+
/**
237+
* @ticket 32429
238+
*/
239+
function test_legacy_user_activation_key_is_rejected() {
240+
global $wpdb;
241+
242+
// A legacy user_activation_key is one without the `time()` prefix introduced in WordPress 4.3.
243+
244+
$key = wp_generate_password( 20, false );
245+
$user = $this->factory->user->create_and_get();
246+
$wpdb->update( $wpdb->users, array(
247+
'user_activation_key' => $this->wp_hasher->HashPassword( $key ),
248+
), array(
249+
'ID' => $user->ID,
250+
) );
251+
252+
// A legacy user_activation_key should not be accepted
253+
$check = check_password_reset_key( $key, $user->user_login );
254+
$this->assertInstanceOf( 'WP_Error', $check );
255+
256+
// An empty key with a legacy user_activation_key should be rejected
257+
$check = check_password_reset_key( '', $user->user_login );
258+
$this->assertInstanceOf( 'WP_Error', $check );
259+
}
260+
261+
/**
262+
* @ticket 32429
263+
* @ticket 24783
264+
*/
265+
function test_plaintext_user_activation_key_is_rejected() {
266+
global $wpdb;
267+
268+
// A plaintext user_activation_key is one stored before hashing was introduced in WordPress 3.7.
269+
270+
$key = wp_generate_password( 20, false );
271+
$user = $this->factory->user->create_and_get();
272+
$wpdb->update( $wpdb->users, array(
273+
'user_activation_key' => $key,
274+
), array(
275+
'ID' => $user->ID,
276+
) );
277+
278+
// A plaintext user_activation_key should not allow an otherwise valid key to be accepted
279+
$check = check_password_reset_key( $key, $user->user_login );
280+
$this->assertInstanceOf( 'WP_Error', $check );
281+
282+
// A plaintext user_activation_key should not allow an empty key to be accepted
283+
$check = check_password_reset_key( '', $user->user_login );
284+
$this->assertInstanceOf( 'WP_Error', $check );
285+
}
162286
}

0 commit comments

Comments
 (0)