Changeset 33019

Timestamp:

07/01/2015 06:32:07 AM (10 years ago)

Author:

dd32

Message:

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

Location:

trunk

Files:

• src/wp-includes/user.php (modified) (2 diffs)
• src/wp-login.php (modified) (2 diffs)
• tests/phpunit/tests/auth.php (modified) (2 diffs)

trunk/src/wp-includes/user.php

@@ -2459 +2459 @@
     }
 
-    if ( $wp_hasher->CheckPassword( $key, $row->user_activation_key ) )
+    /**
+     * Filter the expiration time of password reset keys.
+     *
+     * @since 4.3.0
+     *
+     * @param int $expiration The expiration time in seconds.
+     */
+    $expiration_duration = apply_filters( 'password_reset_expiration', DAY_IN_SECONDS );
+
+    if ( false !== strpos( $row->user_activation_key, ':' ) ) {
+        list( $pass_request_time, $pass_key ) = explode( ':', $row->user_activation_key, 2 );
+        $expiration_time = $pass_request_time + $expiration_duration;
+    } else {
+        $pass_key = $row->user_activation_key;
+        $expiration_time = false;
+    }
+
+    $hash_is_correct = $wp_hasher->CheckPassword( $key, $pass_key );
+
+    if ( $hash_is_correct && $expiration_time && time() < $expiration_time ) {
         return get_userdata( $row->ID );
-
-    if ( $key === $row->user_activation_key ) {
+    } elseif ( $hash_is_correct && $expiration_time ) {
+        // Key has an expiration time that's passed
+        return new WP_Error( 'expired_key', __( 'Invalid key' ) );
+    }
+
+    if ( hash_equals( $row->user_activation_key, $key ) || ( $hash_is_correct && ! $expiration_time ) ) {
         $return = new WP_Error( 'expired_key', __( 'Invalid key' ) );
         $user_id = $row->ID;
@@ -2468 +2491 @@
         /**
          * Filter the return value of check_password_reset_key() when an
-         * old-style key is used (plain-text key was stored in the database).
+         * old-style key is used.
          *
-         * @since 3.7.0
+         * @since 3.7.0 Previously plain-text keys were stored in the database.
+         * @since 4.3.0 Previously key hashes were stored without an expiration time.
          *
          * @param WP_Error $return  A WP_Error object denoting an expired key.

trunk/src/wp-login.php

@@ -364 +364 @@
         $wp_hasher = new PasswordHash( 8, true );
     }
-    $hashed = $wp_hasher->HashPassword( $key );
+    $hashed = time() . ':' . $wp_hasher->HashPassword( $key );
     $wpdb->update( $wpdb->users, array( 'user_activation_key' => $hashed ), array( 'user_login' => $user_login ) );
 
@@ -529 +529 @@
 
     if ( isset( $_GET['error'] ) ) {
-        if ( 'invalidkey' == $_GET['error'] )
-            $errors->add( 'invalidkey', __( 'Sorry, that key does not appear to be valid.' ) );
-        elseif ( 'expiredkey' == $_GET['error'] )
-            $errors->add( 'expiredkey', __( 'Sorry, that key has expired. Please try again.' ) );
+        if ( 'invalidkey' == $_GET['error'] ) {
+            $errors->add( 'invalidkey', __( 'Your password reset link appears to be invalid. Please request a new lnk below.' ) );
+        } elseif ( 'expiredkey' == $_GET['error'] ) {
+            $errors->add( 'expiredkey', __( 'Your password reset link has expired. Please request a new link below.' ) );
+        }
     }
 

trunk/tests/phpunit/tests/auth.php

@@ -7 +7 @@
 class Tests_Auth extends WP_UnitTestCase {
     var $user_id;
+    var $wp_hasher;
 
     function setUp() {
         parent::setUp();
         $this->user_id = $this->factory->user->create();
+
+        require_once ABSPATH . WPINC . '/class-phpass.php';
+        $this->wp_hasher = new PasswordHash( 8, true );
     }
 
@@ -160 +164 @@
         $this->assertInstanceOf( 'WP_Error', $user );
     }
+
+    /**
+     * @ticket 32429
+     */
+    function test_user_activation_key_is_checked() {
+        global $wpdb;
+
+        $key  = wp_generate_password( 20, false );
+        $user = $this->factory->user->create_and_get();
+        $wpdb->update( $wpdb->users, array(
+            'user_activation_key' => strtotime( '-1 hour' ) . ':' . $this->wp_hasher->HashPassword( $key ),
+        ), array(
+            'ID' => $user->ID,
+        ) );
+
+        // A valid key should be accepted
+        $check = check_password_reset_key( $key, $user->user_login );
+        $this->assertInstanceOf( 'WP_User', $check );
+        $this->assertSame( $user->ID, $check->ID );
+
+        // An invalid key should be rejected
+        $check = check_password_reset_key( 'key', $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+
+        // An empty key should be rejected
+        $check = check_password_reset_key( '', $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+
+        // A truncated key should be rejected
+        $partial = substr( $key, 0, 10 );
+        $check = check_password_reset_key( $partial, $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+    }
+
+    /**
+     * @ticket 32429
+     */
+    function test_expired_user_activation_key_is_rejected() {
+        global $wpdb;
+
+        $key  = wp_generate_password( 20, false );
+        $user = $this->factory->user->create_and_get();
+        $wpdb->update( $wpdb->users, array(
+            'user_activation_key' => strtotime( '-48 hours' ) . ':' . $this->wp_hasher->HashPassword( $key ),
+        ), array(
+            'ID' => $user->ID,
+        ) );
+
+        // An expired but otherwise valid key should be rejected
+        $check = check_password_reset_key( $key, $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+    }
+
+    /**
+     * @ticket 32429
+     */
+    function test_empty_user_activation_key_fails_key_check() {
+        global $wpdb;
+
+        $user = $this->factory->user->create_and_get();
+
+        // An empty user_activation_key should not allow any key to be accepted
+        $check = check_password_reset_key( 'key', $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+
+        // An empty user_activation_key should not allow an empty key to be accepted
+        $check = check_password_reset_key( '', $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+    }
+
+    /**
+     * @ticket 32429
+     */
+    function test_legacy_user_activation_key_is_rejected() {
+        global $wpdb;
+
+        // A legacy user_activation_key is one without the `time()` prefix introduced in WordPress 4.3.
+
+        $key  = wp_generate_password( 20, false );
+        $user = $this->factory->user->create_and_get();
+        $wpdb->update( $wpdb->users, array(
+            'user_activation_key' => $this->wp_hasher->HashPassword( $key ),
+        ), array(
+            'ID' => $user->ID,
+        ) );
+
+        // A legacy user_activation_key should not be accepted
+        $check = check_password_reset_key( $key, $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+
+        // An empty key with a legacy user_activation_key should be rejected
+        $check = check_password_reset_key( '', $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+    }
+
+    /**
+     * @ticket 32429
+     * @ticket 24783
+     */
+    function test_plaintext_user_activation_key_is_rejected() {
+        global $wpdb;
+
+        // A plaintext user_activation_key is one stored before hashing was introduced in WordPress 3.7.
+
+        $key  = wp_generate_password( 20, false );
+        $user = $this->factory->user->create_and_get();
+        $wpdb->update( $wpdb->users, array(
+            'user_activation_key' => $key,
+        ), array(
+            'ID' => $user->ID,
+        ) );
+
+        // A plaintext user_activation_key should not allow an otherwise valid key to be accepted
+        $check = check_password_reset_key( $key, $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+
+        // A plaintext user_activation_key should not allow an empty key to be accepted
+        $check = check_password_reset_key( '', $user->user_login );
+        $this->assertInstanceOf( 'WP_Error', $check );
+    }
 }