Page MenuHomePhabricator

0002-T165846-REL1_30.patch

Authored By
Reedy
Nov 2 2017, 11:58 PM
Size
3 KB
Referenced Files
None
Subscribers
None

0002-T165846-REL1_30.patch

From 77254008ec8214e6ec60cc2154a957c2de4216a6 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
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

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5059477
Default Alt Text
0002-T165846-REL1_30.patch (3 KB)

Event Timeline