[5.4] Secure password reset tokens against timing attacks and compromised DB

src/Illuminate/Auth/Passwords/DatabaseTokenRepository.php

@@ -5,6 +5,7 @@
 use Carbon\Carbon;
 use Illuminate\Support\Str;
 use Illuminate\Database\ConnectionInterface;
+use Illuminate\Contracts\Hashing\Hasher as HasherContract;
 use Illuminate\Contracts\Auth\CanResetPassword as CanResetPasswordContract;
 
 class DatabaseTokenRepository implements TokenRepositoryInterface
@@ -37,6 +38,13 @@ class DatabaseTokenRepository implements TokenRepositoryInterface
      */
     protected $expires;
 
+    /**
+     * The hasher implementation.
+     *
+     * @var \Illuminate\Contracts\Hashing\Hasher
+     */
+    protected $hasher;
+
     /**
      * Create a new token repository instance.
      *
@@ -46,12 +54,13 @@ class DatabaseTokenRepository implements TokenRepositoryInterface
      * @param  int  $expires
      * @return void
      */
-    public function __construct(ConnectionInterface $connection, $table, $hashKey, $expires = 60)
+    public function __construct(ConnectionInterface $connection, HasherContract $hasher, $table, $hashKey, $expires = 60)
     {
         $this->table = $table;
         $this->hashKey = $hashKey;
         $this->expires = $expires * 60;
         $this->connection = $connection;
+        $this->hasher = $hasher;
     }
 
     /**
@@ -96,7 +105,7 @@ protected function deleteExisting(CanResetPasswordContract $user)
      */
     protected function getPayload($email, $token)
     {
-        return ['email' => $email, 'token' => $token, 'created_at' => new Carbon];
+        return ['email' => $email, 'token' => $this->hasher->make($token), 'created_at' => new Carbon];
     }
 
     /**
@@ -106,13 +115,13 @@ protected function getPayload($email, $token)
      * @param  string  $token
      * @return bool
      */
-    public function exists(CanResetPasswordContract $user, $token)
+    public function exists(CanResetPasswordContract $user, $userToken)
     {
         $email = $user->getEmailForPasswordReset();
 
-        $token = (array) $this->getTable()->where('email', $email)->where('token', $token)->first();
+        $token = (array) $this->getTable()->where('email', $email)->first();
 
-        return $token && ! $this->tokenExpired($token);
+        return $token && ! $this->tokenExpired($token) && $this->hasher->check($userToken, $token['token']);
     }
 
     /**
@@ -180,4 +189,14 @@ public function getConnection()
     {
         return $this->connection;
     }
+
+    /**
+     * Get the hasher instance.
+     *
+     * @return \Illuminate\Contracts\Hashing\Hasher
+     */
+    public function getHasher()
+    {
+        return $this->hasher;
+    }
 }

src/Illuminate/Auth/Passwords/PasswordBrokerManager.php

@@ -91,6 +91,7 @@ protected function createTokenRepository(array $config)
 
         return new DatabaseTokenRepository(
             $this->app['db']->connection($connection),
+            $this->app['hash'],
             $config['table'],
             $key,
             $config['expire']

tests/Auth/AuthDatabaseTokenRepositoryTest.php

@@ -12,6 +12,7 @@ public function tearDown()
     public function testCreateInsertsNewRecordIntoTable()
     {
         $repo = $this->getRepo();
+        $repo->getHasher()->shouldReceive('make')->andReturn('hashed-token');
         $repo->getConnection()->shouldReceive('table')->with('table')->andReturn($query = m::mock('StdClass'));
         $query->shouldReceive('where')->with('email', 'email')->andReturn($query);
         $query->shouldReceive('delete')->once();
@@ -30,7 +31,6 @@ public function testExistReturnsFalseIfNoRowFoundForUser()
         $repo = $this->getRepo();
         $repo->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($query = m::mock('StdClass'));
         $query->shouldReceive('where')->once()->with('email', 'email')->andReturn($query);
-        $query->shouldReceive('where')->once()->with('token', 'token')->andReturn($query);
         $query->shouldReceive('first')->andReturn(null);
         $user = m::mock('Illuminate\Contracts\Auth\CanResetPassword');
         $user->shouldReceive('getEmailForPasswordReset')->andReturn('email');
@@ -41,11 +41,11 @@ public function testExistReturnsFalseIfNoRowFoundForUser()
     public function testExistReturnsFalseIfRecordIsExpired()
     {
         $repo = $this->getRepo();
+        $repo->getHasher()->shouldReceive('check')->with('token', 'hashed-token')->andReturn(true);
         $repo->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($query = m::mock('StdClass'));
         $query->shouldReceive('where')->once()->with('email', 'email')->andReturn($query);
-        $query->shouldReceive('where')->once()->with('token', 'token')->andReturn($query);
         $date = date('Y-m-d H:i:s', time() - 300000);
-        $query->shouldReceive('first')->andReturn((object) ['created_at' => $date]);
+        $query->shouldReceive('first')->andReturn((object) ['created_at' => $date, 'token' => 'hashed-token']);
         $user = m::mock('Illuminate\Contracts\Auth\CanResetPassword');
         $user->shouldReceive('getEmailForPasswordReset')->andReturn('email');
 
@@ -55,17 +55,31 @@ public function testExistReturnsFalseIfRecordIsExpired()
     public function testExistReturnsTrueIfValidRecordExists()
     {
         $repo = $this->getRepo();
+        $repo->getHasher()->shouldReceive('check')->with('token', 'hashed-token')->andReturn(true);
         $repo->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($query = m::mock('StdClass'));
         $query->shouldReceive('where')->once()->with('email', 'email')->andReturn($query);
-        $query->shouldReceive('where')->once()->with('token', 'token')->andReturn($query);
         $date = date('Y-m-d H:i:s', time() - 600);
-        $query->shouldReceive('first')->andReturn((object) ['created_at' => $date]);
+        $query->shouldReceive('first')->andReturn((object) ['created_at' => $date, 'token' => 'hashed-token']);
         $user = m::mock('Illuminate\Contracts\Auth\CanResetPassword');
         $user->shouldReceive('getEmailForPasswordReset')->andReturn('email');
 
         $this->assertTrue($repo->exists($user, 'token'));
     }
 
+    public function testExistReturnsFalseIfInvalidToken()
+    {
+        $repo = $this->getRepo();
+        $repo->getHasher()->shouldReceive('check')->with('wrong-token', 'hashed-token')->andReturn(false);
+        $repo->getConnection()->shouldReceive('table')->once()->with('table')->andReturn($query = m::mock('StdClass'));
+        $query->shouldReceive('where')->once()->with('email', 'email')->andReturn($query);
+        $date = date('Y-m-d H:i:s', time() - 600);
+        $query->shouldReceive('first')->andReturn((object) ['created_at' => $date, 'token' => 'hashed-token']);
+        $user = m::mock('Illuminate\Contracts\Auth\CanResetPassword');
+        $user->shouldReceive('getEmailForPasswordReset')->andReturn('email');
+
+        $this->assertFalse($repo->exists($user, 'wrong-token'));
+    }
+
     public function testDeleteMethodDeletesByToken()
     {
         $repo = $this->getRepo();
@@ -88,6 +102,9 @@ public function testDeleteExpiredMethodDeletesExpiredTokens()
 
     protected function getRepo()
     {
-        return new Illuminate\Auth\Passwords\DatabaseTokenRepository(m::mock('Illuminate\Database\Connection'), 'table', 'key');
+        return new Illuminate\Auth\Passwords\DatabaseTokenRepository(
+            m::mock('Illuminate\Database\Connection'),
+            m::mock('Illuminate\Contracts\Hashing\Hasher'),
+            'table', 'key');
     }
 }