Add password max size of 4096 bytes

ar-rohman · May 1, 2018 · 8185e46 · 8185e46
1 parent 1b6a632
commit 8185e46
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,8 @@ Ion Auth Changelog
 
 ## xx March 2018 - Ion Auth 3
 
+ - **General:**
+    - No longer work for empty password or password above 4096 bytes (DOS protection)
  - **New server requirements:**
     - Drop CodeIgniter 2 support
     - Drop PHP < 5.6 support

diff --git a/config/ion_auth.php b/config/ion_auth.php
@@ -126,6 +126,9 @@
  | min_password_length:		This minimum is not enforced directly by the library.
  | 							The controller should define a validation rule to enforce it.
  | 							See the Auth controller for an example implementation.
+ |
+ | The library will fail for empty password or password size above 4096 bytes.
+ | This is an arbitrary (long) value to protect against DOS attack.
  */
 $config['site_title']                 = "Example.com";       // Site Title, example.com
 $config['admin_email']                = "[email protected]"; // Admin Email, [email protected]

diff --git a/models/Ion_auth_model.php b/models/Ion_auth_model.php
@@ -32,6 +32,11 @@ class Ion_auth_model extends CI_Model
 	 */
 	const MAX_COOKIE_LIFETIME = 63072000; // 2 years = 60*60*24*365*2 = 63072000 seconds;
 
+    /**
+     * Max password size constant
+     */
+    const MAX_PASSWORD_SIZE_BYTES = 4096;
+
 	/**
 	 * Holds an array of tables used
 	 *
@@ -259,9 +264,11 @@ public function db()
 	 */
 	public function hash_password($password, $identity = NULL)
 	{
-		// Check for empty password, or password containing null char
+		// Check for empty password, or password containing null char, or password above limit
 		// Null char may pose issue: http://php.net/manual/en/function.password-hash.php#118603
-		if (empty($password) || strpos($password, "\0") !== FALSE)
+        // Long password may pose DOS issue (note: strlen gives size in bytes and not in multibyte symbol)
+		if (empty($password) || strpos($password, "\0") !== FALSE ||
+            strlen($password) > self::MAX_PASSWORD_SIZE_BYTES)
 		{
 			return FALSE;
 		}
@@ -290,9 +297,11 @@ public function hash_password($password, $identity = NULL)
 	 */
 	public function verify_password($password, $hash_password_db, $identity = NULL)
 	{
-		// Check for empty id or password, or password containing null char
+		// Check for empty id or password, or password containing null char, or password above limit
 		// Null char may pose issue: http://php.net/manual/en/function.password-hash.php#118603
-		if (empty($password) || empty($hash_password_db) || strpos($password, "\0") !== FALSE)
+        // Long password may pose DOS issue (note: strlen gives size in bytes and not in multibyte symbol)
+		if (empty($password) || empty($hash_password_db) || strpos($password, "\0") !== FALSE
+            || strlen($password) > self::MAX_PASSWORD_SIZE_BYTES)
 		{
 			return FALSE;
 		}
@@ -807,6 +816,12 @@ public function register($identity, $password, $email, $additional_data = [], $g
 		// Do not pass $identity as user is not known yet so there is no need
 		$password = $this->hash_password($password);
 
+        if ($password === FALSE)
+        {
+            $this->set_error('account_creation_unsuccessful');
+            return FALSE;
+        }
+
 		// Users table.
 		$data = [
 			$this->identity_column => $identity,
@@ -1762,6 +1777,14 @@ public function update($id, array $data)
 				if( ! empty($data['password']))
 				{
 					$data['password'] = $this->hash_password($data['password'], $user->{$this->identity_column});
+                    if ($data['password'] === FALSE)
+                    {
+                        $this->db->trans_rollback();
+                        $this->trigger_events(['post_update_user', 'post_update_user_unsuccessful']);
+                        $this->set_error('update_unsuccessful');
+
+                        return FALSE;
+                    }
 				}
 				else
 				{
@@ -2466,7 +2489,12 @@ public function clear_errors()
 	 */
 	protected function _set_password_db($identity, $password)
 	{
-		$hash  = $this->hash_password($password, $identity);
+		$hash = $this->hash_password($password, $identity);
+
+        if ($hash === FALSE)
+        {
+            return FALSE;
+        }
 
 		// When setting a new password, invalidate any other token
 		$data = [

diff --git a/userguide/index.html b/userguide/index.html
@@ -306,11 +306,12 @@ <h3>Authentication options</h3>
         </li>
         <li>
             <strong>min_password_length</strong> Default is '8'<br/>
-            Minimum length of passwords.
-        </li>
-        <li>
-            <strong>max_password_length</strong> Default is '20'<br/>
-            Maximum length of passwords.
+            Minimum length of passwords.<br/>
+            This minimum is not enforced directly by the library.<br/>
+            The controller should define a validation rule to enforce it.<br/>
+            See the Auth controller for an example implementation.<br/><br/>
+            Additional note: the library will fail for empty password or password size above 4096 bytes.<br/>
+            This is an arbitrary (long) value to protect against DOS attack.
         </li>
         <li>
             <strong>email_activation</strong> Default is 'false'<br/>