From 77254008ec8214e6ec60cc2154a957c2de4216a6 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Fri, 19 May 2017 23:35:11 +0200 Subject: [PATCH] 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 --- RELEASE-NOTES-1.30 | 1 + includes/api/ApiLogin.php | 2 +- includes/user/BotPassword.php | 19 ++++++++++++++++++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/RELEASE-NOTES-1.30 b/RELEASE-NOTES-1.30 index 3332b5d145..e65f696fe0 100644 --- a/RELEASE-NOTES-1.30 +++ b/RELEASE-NOTES-1.30 @@ -246,6 +246,7 @@ changes to languages because of Phabricator reports. should be used instead. * (T178451) SECURITY: Potential XSS when $wgShowExceptionDetails = false and browser sends non-standard url escaping. +* (T165846) SECURITY: BotPassword login attempts weren't throttled. == Compatibility == MediaWiki 1.30 requires PHP 5.5.9 or later. There is experimental support for diff --git a/includes/api/ApiLogin.php b/includes/api/ApiLogin.php index aa7e25e046..9636789fbb 100644 --- a/includes/api/ApiLogin.php +++ b/includes/api/ApiLogin.php @@ -134,7 +134,7 @@ class ApiLogin extends ApiBase { $session = $status->getValue(); $authRes = 'Success'; $loginType = 'BotPassword'; - } elseif ( !$botLoginData[2] ) { + } elseif ( !$botLoginData[2] || $status->hasMessage( 'login-throttled' ) ) { $authRes = 'Failed'; $message = $status->getMessage(); LoggerFactory::getInstance( 'authentication' )->info( diff --git a/includes/user/BotPassword.php b/includes/user/BotPassword.php index 25625e72c8..b898d8a5da 100644 --- a/includes/user/BotPassword.php +++ b/includes/user/BotPassword.php @@ -437,7 +437,7 @@ class BotPassword implements IDBAccessObject { * @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' ); @@ -462,6 +462,20 @@ class BotPassword implements IDBAccessObject { 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 ) { @@ -480,6 +494,9 @@ class BotPassword implements IDBAccessObject { } // Ok! Create the session. + if ( $throttle ) { + $throttle->clear( $user->getName(), $request->getIP() ); + } return Status::newGood( $provider->newSessionForRequest( $user, $bp, $request ) ); } } -- 2.14.1