[Security] limited the password length passed to encoders

symfony · Oct 10, 2013 · 13d7d3a · 13d7d3a
1 parent 2f19dab
commit 13d7d3a
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 1 deletion.
diff --git a/src/Symfony/Component/Security/Core/Encoder/BasePasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/BasePasswordEncoder.php
@@ -18,6 +18,8 @@
  */
 abstract class BasePasswordEncoder implements PasswordEncoderInterface
 {
+    const MAX_PASSWORD_LENGTH = 4096;
+
     /**
      * Demerges a merge password and salt string.
      *
@@ -88,4 +90,14 @@ protected function comparePasswords($password1, $password2)
 
         return 0 === $result;
     }
+
+    /**
+     * Checks if the password is too long.
+     *
+     * @return Boolean true if the password is too long, false otherwise
+     */
+    protected function isPasswordTooLong($password)
+    {
+        return strlen($password) > self::MAX_PASSWORD_LENGTH;
+    }
 }
diff --git a/src/Symfony/Component/Security/Core/Encoder/MessageDigestPasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/MessageDigestPasswordEncoder.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Security\Core\Encoder;
 
+use Symfony\Component\Security\Core\Exception\BadCredentialsException;
+
 /**
  * MessageDigestPasswordEncoder uses a message digest algorithm.
  *
@@ -40,6 +42,10 @@ public function __construct($algorithm = 'sha512', $encodeHashAsBase64 = true, $
      */
     public function encodePassword($raw, $salt)
     {
+        if ($this->isPasswordTooLong($raw)) {
+            throw new BadCredentialsException('Invalid password.');
+        }
+
         if (!in_array($this->algorithm, hash_algos(), true)) {
             throw new \LogicException(sprintf('The algorithm "%s" is not supported.', $this->algorithm));
         }
@@ -60,6 +66,6 @@ public function encodePassword($raw, $salt)
      */
     public function isPasswordValid($encoded, $raw, $salt)
     {
-        return $this->comparePasswords($encoded, $this->encodePassword($raw, $salt));
+        return !$this->isPasswordTooLong($raw) && $this->comparePasswords($encoded, $this->encodePassword($raw, $salt));
     }
 }
diff --git a/src/Symfony/Component/Security/Core/Encoder/PlaintextPasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/PlaintextPasswordEncoder.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Security\Core\Encoder;
 
+use Symfony\Component\Security\Core\Exception\BadCredentialsException;
+
 /**
  * PlaintextPasswordEncoder does not do any encoding.
  *
@@ -30,6 +32,10 @@ public function __construct($ignorePasswordCase = false)
      */
     public function encodePassword($raw, $salt)
     {
+        if ($this->isPasswordTooLong($raw)) {
+            throw new BadCredentialsException('Invalid password.');
+        }
+
         return $this->mergePasswordAndSalt($raw, $salt);
     }
 
@@ -38,6 +44,10 @@ public function encodePassword($raw, $salt)
      */
     public function isPasswordValid($encoded, $raw, $salt)
     {
+        if ($this->isPasswordTooLong($raw)) {
+            return false;
+        }
+
         $pass2 = $this->mergePasswordAndSalt($raw, $salt);
 
         if (!$this->ignorePasswordCase) {

diff --git a/tests/Symfony/Tests/Component/Security/Core/Encoder/BasePasswordEncoderTest.php b/tests/Symfony/Tests/Component/Security/Core/Encoder/BasePasswordEncoderTest.php
@@ -53,6 +53,12 @@ public function testMergePasswordAndSaltWithException()
         $this->invokeMergePasswordAndSalt('password', '{foo}');
     }
 
+    public function testIsPasswordTooLong()
+    {
+        $this->assertTrue($this->invokeIsPasswordTooLong(str_repeat('a', 10000)));
+        $this->assertFalse($this->invokeIsPasswordTooLong(str_repeat('a', 10)));
+    }
+
     protected function invokeDemergePasswordAndSalt($password)
     {
         $encoder = new PasswordEncoder();
@@ -82,4 +88,14 @@ protected function invokeComparePasswords($p1, $p2)
 
         return $m->invoke($encoder, $p1, $p2);
     }
+
+    protected function invokeIsPasswordTooLong($p)
+    {
+        $encoder = new PasswordEncoder();
+        $r = new \ReflectionObject($encoder);
+        $m = $r->getMethod('isPasswordTooLong');
+        $m->setAccessible(true);
+
+        return $m->invoke($encoder, $p);
+    }
 }
diff --git a/tests/Symfony/Tests/Component/Security/Core/Encoder/MessageDigestPasswordEncoderTest.php b/tests/Symfony/Tests/Component/Security/Core/Encoder/MessageDigestPasswordEncoderTest.php
@@ -42,4 +42,21 @@ public function testEncodePasswordAlgorithmDoesNotExist()
         $encoder = new MessageDigestPasswordEncoder('foobar');
         $encoder->encodePassword('password', '');
     }
+
+    /**
+     * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException
+     */
+    public function testEncodePasswordLength()
+    {
+        $encoder = new MessageDigestPasswordEncoder();
+
+        $encoder->encodePassword(str_repeat('a', 5000), 'salt');
+    }
+
+    public function testCheckPasswordLength()
+    {
+        $encoder = new MessageDigestPasswordEncoder();
+
+        $this->assertFalse($encoder->isPasswordValid('encoded', str_repeat('a', 5000), 'salt'));
+    }
 }
diff --git a/tests/Symfony/Tests/Component/Security/Core/Encoder/PlaintextPasswordEncoderTest.php b/tests/Symfony/Tests/Component/Security/Core/Encoder/PlaintextPasswordEncoderTest.php
@@ -36,4 +36,21 @@ public function testEncodePassword()
 
         $this->assertSame('foo', $encoder->encodePassword('foo', ''));
     }
+
+    /**
+     * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException
+     */
+    public function testEncodePasswordLength()
+    {
+        $encoder = new PlaintextPasswordEncoder();
+
+        $encoder->encodePassword(str_repeat('a', 5000), 'salt');
+    }
+
+    public function testCheckPasswordLength()
+    {
+        $encoder = new PlaintextPasswordEncoder();
+
+        $this->assertFalse($encoder->isPasswordValid('encoded', str_repeat('a', 5000), 'salt'));
+    }
 }