Ejegg has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/391434 )

Change subject: SECURITY: Add throttling for BotPasswords authentication 
attempts
......................................................................


SECURITY: Add throttling for BotPasswords authentication attempts

ApiLogin which will currently always try an AuthManager login which will
by default throttle via ThrottlePreAuthenticationProvider, but this only
happens after the BotPassword is checked so it's still possible to keep
trying to break the bot password.

There's a potential odd-behavior mode here: if the main account username
and password looks like a BotPasswords username and password, a
successful main account login will increment the BotPasswords throttle
for the user and not reset it after the successful main account login.
That seems such an odd edge case I say let's not worry about it.

Bug: T165846
Change-Id: Ie60f0e05c2a94722b91bc3a80c80346e28b443f4
---
M RELEASE-NOTES-1.27
M includes/api/ApiLogin.php
M includes/user/BotPassword.php
3 files changed, 21 insertions(+), 2 deletions(-)

Approvals:
  Ejegg: Verified; Looks good to me, approved



diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27
index 4485572..007618f 100644
--- a/RELEASE-NOTES-1.27
+++ b/RELEASE-NOTES-1.27
@@ -19,6 +19,8 @@
 * (T142304) Allow putting the app ID in the password for bot passwords.
 * (T178451) SECURITY: Potential XSS when $wgShowExceptionDetails = false and 
browser
   sends non-standard url escaping.
+* (T165846) SECURITY: BotPassword login attempts weren't throttled.
+* Updated dev dependancy phpunit/phpunit from v4.8.24 to v4.8.36.
 
 == MediaWiki 1.27.3 ==
 Due to a packaging error, the wrong version of the SyntaxHighlight extension 
was
diff --git a/includes/api/ApiLogin.php b/includes/api/ApiLogin.php
index be2d93e..0df9057 100644
--- a/includes/api/ApiLogin.php
+++ b/includes/api/ApiLogin.php
@@ -121,7 +121,7 @@
                                $session = $status->getValue();
                                $authRes = 'Success';
                                $loginType = 'BotPassword';
-                       } elseif ( !$botLoginData[2] ) {
+                       } elseif ( !$botLoginData[2] || $status->hasMessage( 
'login-throttled' ) ) {
                                $authRes = 'Failed';
                                $message = $status->getMessage();
                                LoggerFactory::getInstance( 'authmanager' 
)->info(
diff --git a/includes/user/BotPassword.php b/includes/user/BotPassword.php
index 80fa7f3..2b38559 100644
--- a/includes/user/BotPassword.php
+++ b/includes/user/BotPassword.php
@@ -436,7 +436,7 @@
         * @return Status On success, the good status's value is the new 
Session object
         */
        public static function login( $username, $password, WebRequest $request 
) {
-               global $wgEnableBotPasswords;
+               global $wgEnableBotPasswords, $wgPasswordAttemptThrottle;
 
                if ( !$wgEnableBotPasswords ) {
                        return Status::newFatal( 'botpasswords-disabled' );
@@ -461,6 +461,20 @@
                        return Status::newFatal( 'nosuchuser', $name );
                }
 
+               // Throttle
+               $throttle = null;
+               if ( !empty( $wgPasswordAttemptThrottle ) ) {
+                       $throttle = new MediaWiki\Auth\Throttler( 
$wgPasswordAttemptThrottle, [
+                               'type' => 'botpassword',
+                               'cache' => 
ObjectCache::getLocalClusterInstance(),
+                       ] );
+                       $result = $throttle->increase( $user->getName(), 
$request->getIP(), __METHOD__ );
+                       if ( $result ) {
+                               $msg = wfMessage( 'login-throttled' 
)->durationParams( $result['wait'] );
+                               return Status::newFatal( $msg );
+                       }
+               }
+
                // Get the bot password
                $bp = self::newFromUser( $user, $appId );
                if ( !$bp ) {
@@ -479,6 +493,9 @@
                }
 
                // Ok! Create the session.
+               if ( $throttle ) {
+                       $throttle->clear( $user->getName(), $request->getIP() );
+               }
                return Status::newGood( $provider->newSessionForRequest( $user, 
$bp, $request ) );
        }
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/391434
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie60f0e05c2a94722b91bc3a80c80346e28b443f4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: fundraising/REL1_27
Gerrit-Owner: Ejegg <ej...@ejegg.com>
Gerrit-Reviewer: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: Ejegg <ej...@ejegg.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to