SECURITY: Set maximal password length for DoS

[lhc/web/wiklou.git] / includes / User.php

diff --git a/includes/User.php b/includes/User.php
index 7f30263..2e88978 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -826,15 +826,24 @@ class User implements IDBAccessObject {
        }
 
        /**
-        * Check if this is a valid password for this user. Status will be good if
-        * the password is valid, or have an array of error messages if not.
+        * Check if this is a valid password for this user
+        *
+        * Create a Status object based on the password's validity.
+        * The Status should be set to fatal if the user should not
+        * be allowed to log in, and should have any errors that
+        * would block changing the password.
+        *
+        * If the return value of this is not OK, the password
+        * should not be checked. If the return value is not Good,
+        * the password can be checked, but the user should not be
+        * able to set their password to this.
         *
         * @param string $password Desired password
         * @return Status
         * @since 1.23
         */
        public function checkPasswordValidity( $password ) {
-               global $wgMinimalPasswordLength, $wgContLang;
+               global $wgMinimalPasswordLength, $wgMaximalPasswordLength, $wgContLang;
 
                static $blockedLogins = array(
                        'Useruser' => 'Passpass', 'Useruser1' => 'Passpass1', # r75589
@@ -854,6 +863,10 @@ class User implements IDBAccessObject {
                        if ( strlen( $password ) < $wgMinimalPasswordLength ) {
                                $status->error( 'passwordtooshort', $wgMinimalPasswordLength );
                                return $status;
+                       } elseif ( strlen( $password ) > $wgMaximalPasswordLength ) {
+                               // T64685: Password too long, might cause DoS attack
+                               $status->fatal( 'passwordtoolong', $wgMaximalPasswordLength );
+                               return $status;
                        } elseif ( $wgContLang->lc( $password ) == $wgContLang->lc( $this->mName ) ) {
                                $status->error( 'password-name-match' );
                                return $status;
@@ -2119,17 +2132,13 @@ class User implements IDBAccessObject {
         * @see getNewtalk()
         * @param string $field 'user_ip' for anonymous users, 'user_id' otherwise
         * @param string|int $id User's IP address for anonymous users, User ID otherwise
-        * @param bool $fromMaster True to fetch from the master, false for a slave
         * @return bool True if the user has new messages
         */
-       protected function checkNewtalk( $field, $id, $fromMaster = false ) {
-               if ( $fromMaster ) {
-                       $db = wfGetDB( DB_MASTER );
-               } else {
-                       $db = wfGetDB( DB_SLAVE );
-               }
-               $ok = $db->selectField( 'user_newtalk', $field,
-                       array( $field => $id ), __METHOD__ );
+       protected function checkNewtalk( $field, $id ) {
+               $dbr = wfGetDB( DB_SLAVE );
+
+               $ok = $dbr->selectField( 'user_newtalk', $field, array( $field => $id ), __METHOD__ );
+
                return $ok !== false;
        }
 
@@ -2386,17 +2395,9 @@ class User implements IDBAccessObject {
                                throw new PasswordError( wfMessage( 'password-change-forbidden' )->text() );
                        }
 
-                       if ( !$this->isValidPassword( $str ) ) {
-                               global $wgMinimalPasswordLength;
-                               $valid = $this->getPasswordValidity( $str );
-                               if ( is_array( $valid ) ) {
-                                       $message = array_shift( $valid );
-                                       $params = $valid;
-                               } else {
-                                       $message = $valid;
-                                       $params = array( $wgMinimalPasswordLength );
-                               }
-                               throw new PasswordError( wfMessage( $message, $params )->text() );
+                       $status = $this->checkPasswordValidity( $str );
+                       if ( !$status->isGood() ) {
+                               throw new PasswordError( $status->getMessage()->text() );
                        }
                }
 
@@ -3038,8 +3039,12 @@ class User implements IDBAccessObject {
         * @return array Names of the groups the user has belonged to.
         */
        public function getFormerGroups() {
+               $this->load();
+
                if ( is_null( $this->mFormerGroups ) ) {
-                       $dbr = wfGetDB( DB_MASTER );
+                       $dbr = ( $this->queryFlagsUsed & self::READ_LATEST )
+                               ? wfGetDB( DB_MASTER )
+                               : wfGetDB( DB_SLAVE );
                        $res = $dbr->select( 'user_former_groups',
                                array( 'ufg_group' ),
                                array( 'ufg_user' => $this->mId ),
@@ -3049,6 +3054,7 @@ class User implements IDBAccessObject {
                                $this->mFormerGroups[] = $row->ufg_group;
                        }
                }
+
                return $this->mFormerGroups;
        }
 
@@ -3899,6 +3905,13 @@ class User implements IDBAccessObject {
 
                $this->loadPasswords();
 
+               // Some passwords will give a fatal Status, which means there is
+               // some sort of technical or security reason for this password to
+               // be completely invalid and should never be checked (e.g., T64685)
+               if ( !$this->checkPasswordValidity( $password )->isOK() ) {
+                       return false;
+               }
+
                // Certain authentication plugins do NOT want to save
                // domain passwords in a mysql database, so we should
                // check this (in case $wgAuth->strict() is false).