SECURITY: Set maximal password length for DoS
[lhc/web/wiklou.git] / includes / User.php
index 78693c1..2e88978 100644 (file)
@@ -297,6 +297,9 @@ class User implements IDBAccessObject {
        /** @var array */
        private $mWatchedItems = array();
 
+       /** @var integer User::READ_* constant bitfield used to load data */
+       protected $queryFlagsUsed = self::READ_NORMAL;
+
        public static $idCacheByName = array();
 
        /**
@@ -332,6 +335,7 @@ class User implements IDBAccessObject {
 
                // Set it now to avoid infinite recursion in accessors
                $this->mLoadedItems = true;
+               $this->queryFlagsUsed = $flags;
 
                switch ( $this->mFrom ) {
                        case 'defaults':
@@ -392,6 +396,7 @@ class User implements IDBAccessObject {
                }
 
                $this->mLoadedItems = true;
+               $this->queryFlagsUsed = $flags;
 
                return true;
        }
@@ -402,7 +407,7 @@ class User implements IDBAccessObject {
         * @return bool false if the ID does not exist or data is invalid, true otherwise
         * @since 1.25
         */
-       public function loadFromCache() {
+       protected function loadFromCache() {
                global $wgMemc;
 
                if ( $this->mId == 0 ) {
@@ -429,6 +434,8 @@ class User implements IDBAccessObject {
 
        /**
         * Save user data to the shared cache
+        *
+        * This method should not be called outside the User class
         */
        public function saveToCache() {
                global $wgMemc;
@@ -436,10 +443,19 @@ class User implements IDBAccessObject {
                $this->load();
                $this->loadGroups();
                $this->loadOptions();
+
                if ( $this->isAnon() ) {
                        // Anonymous users are uncached
                        return;
                }
+
+               // The cache needs good consistency due to its high TTL, so the user
+               // should have been loaded from the master to avoid lag amplification.
+               if ( !( $this->queryFlagsUsed & self::READ_LATEST ) ) {
+                       wfWarn( "Cannot save slave-loaded User object data to cache." );
+                       return;
+               }
+
                $data = array();
                foreach ( self::$mCacheVars as $name ) {
                        $data[$name] = $this->$name;
@@ -810,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
@@ -838,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;
@@ -1061,7 +1090,6 @@ class User implements IDBAccessObject {
                $this->mGroups = array();
 
                Hooks::run( 'UserLoadDefaults', array( $this, $name ) );
-
        }
 
        /**
@@ -1205,6 +1233,7 @@ class User implements IDBAccessObject {
                                : array()
                );
 
+               $this->queryFlagsUsed = $flags;
                Hooks::run( 'UserLoadFromDatabase', array( $this, &$s ) );
 
                if ( $s !== false ) {
@@ -1342,7 +1371,9 @@ class User implements IDBAccessObject {
         */
        private function loadGroups() {
                if ( is_null( $this->mGroups ) ) {
-                       $dbr = wfGetDB( DB_MASTER );
+                       $dbr = ( $this->queryFlagsUsed & self::READ_LATEST )
+                               ? wfGetDB( DB_MASTER )
+                               : wfGetDB( DB_SLAVE );
                        $res = $dbr->select( 'user_groups',
                                array( 'ug_group' ),
                                array( 'ug_user' => $this->mId ),
@@ -1366,11 +1397,11 @@ class User implements IDBAccessObject {
        private function loadPasswords() {
                if ( $this->getId() !== 0 && ( $this->mPassword === null || $this->mNewpassword === null ) ) {
                        $this->loadFromRow( wfGetDB( DB_MASTER )->selectRow(
-                                       'user',
-                                       array( 'user_password', 'user_newpassword', 'user_newpass_time', 'user_password_expires' ),
-                                       array( 'user_id' => $this->getId() ),
-                                       __METHOD__
-                               ) );
+                               'user',
+                               array( 'user_password', 'user_newpassword', 'user_newpass_time', 'user_password_expires' ),
+                               array( 'user_id' => $this->getId() ),
+                               __METHOD__
+                       ) );
                }
        }
 
@@ -2101,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;
        }
 
@@ -2292,8 +2319,7 @@ class User implements IDBAccessObject {
         * @return bool
         */
        public function validateCache( $timestamp ) {
-               $this->load();
-               return ( $timestamp >= $this->mTouched );
+               return ( $timestamp >= $this->getTouched() );
        }
 
        /**
@@ -2369,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() );
                        }
                }
 
@@ -3021,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 ),
@@ -3032,6 +3054,7 @@ class User implements IDBAccessObject {
                                $this->mFormerGroups[] = $row->ufg_group;
                        }
                }
+
                return $this->mFormerGroups;
        }
 
@@ -3070,6 +3093,8 @@ class User implements IDBAccessObject {
         * @return bool
         */
        public function addGroup( $group ) {
+               $this->load();
+
                if ( !Hooks::run( 'UserAddGroup', array( $this, &$group ) ) ) {
                        return false;
                }
@@ -3559,12 +3584,18 @@ class User implements IDBAccessObject {
                $this->load();
                $this->loadPasswords();
                if ( wfReadOnly() ) {
-                       return;
+                       return; // @TODO: caller should deal with this instead!
                }
                if ( 0 == $this->mId ) {
                        return;
                }
 
+               // This method is for updating existing users, so the user should
+               // have been loaded from the master to begin with to avoid problems.
+               if ( !( $this->queryFlagsUsed & self::READ_LATEST ) ) {
+                       wfWarn( "Attempting to save slave-loaded User object data." );
+               }
+
                $this->mTouched = self::newTouchedTimestamp();
                if ( !$wgAuth->allowSetLocalPassword() ) {
                        $this->mPassword = self::getPasswordFactory()->newFromCiphertext( null );
@@ -3740,7 +3771,7 @@ class User implements IDBAccessObject {
                                // using CentralAuth. It's should be OK to commit and break the snapshot.
                                $dbw->commit( __METHOD__, 'flush' );
                                $options = array();
-                               $flags = 0;
+                               $flags = self::READ_LATEST;
                        }
                        $this->mId = $dbw->selectField( 'user', 'user_id',
                                array( 'user_name' => $this->mName ), __METHOD__, $options );
@@ -3874,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).
@@ -4911,7 +4949,9 @@ class User implements IDBAccessObject {
                        if ( !is_array( $data ) ) {
                                wfDebug( "User: loading options for user " . $this->getId() . " from database.\n" );
                                // Load from database
-                               $dbr = wfGetDB( DB_SLAVE );
+                               $dbr = ( $this->queryFlagsUsed & self::READ_LATEST )
+                                       ? wfGetDB( DB_MASTER )
+                                       : wfGetDB( DB_SLAVE );
 
                                $res = $dbr->select(
                                        'user_properties',