Skip to content
Permalink
Browse files Browse the repository at this point in the history
Mitigate h1 report 96115
Improper Restriction of Excessive Authentication Attempts
---------------------------------------------------------

Karan M. Tank and Smit B. Shah have reported via HackerOne that the login
page of Revive Adserver was vulnerable to password-guessing attacks. An
account lockdown feature was taken into account, but rejected to avoid
introducing service disruptions to regular users during such attacks.
A random delay has been introduced as a counter-measure in case of password
failures, along with a system to discourage parallel brute forcing. Such
system will effectively allow the actual user to log in to the adserver even
while an attack is in progress.

A CVE-ID has been requested, but not assigned yet.

CWE: CWE-307
CVSSv2: 8.5 (AV:N/AC:L/Au:N/C:C/I:P/A:N)
  • Loading branch information
mbeccati committed Feb 10, 2016
1 parent 0e79cd6 commit 8479413
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 5 deletions.
1 change: 0 additions & 1 deletion lib/OA/Auth.php
Expand Up @@ -83,7 +83,6 @@ function login($redirectCallback = null)
$doUser = OA_Auth::authenticateUser();

if (!$doUser) {
sleep(3);
OA_Auth::restart($GLOBALS['strUsernameOrPasswordWrong']);
}

Expand Down
51 changes: 47 additions & 4 deletions lib/OX/Extension/authentication/authentication.php
Expand Up @@ -15,6 +15,7 @@
require_once 'Date.php';
require_once MAX_PATH . '/lib/max/language/Loader.php';
require_once MAX_PATH . '/lib/pear/HTML/QuickForm/Rule/Email.php';
require_once MAX_PATH . '/lib/OA/DB/AdvisoryLock.php';

Language_Loader::load('settings');

Expand Down Expand Up @@ -98,16 +99,55 @@ function _getCredentials($performCookieCheck = true)
*/
function checkPassword($username, $password)
{
// Introduce a random delay in case of failures, as recommended by:
// https://www.owasp.org/index.php/Blocking_Brute_Force_Attacks
//
// The base delay is 1-5 seconds.
$waitMs = mt_rand(1000, 5000);

$oLock = OA_DB_AdvisoryLock::factory();

// Username check is case insensitive
$username = strtolower($username);

// Try to acquire an excusive advisory lock for the username
$lock = $oLock->get('auth.'.md5($username));

if (!$lock) {
// We couldn't acquire the lock immediately, which means that
// another authentication process for the same username is underway.
//
// This might mean that the account is being targeted by a
// multi-threaded brute force attack, so we try to discourage such
// behaviour by increasing the delay time by 4x.
//
// However, if the actual user tries to log in while their account
// is being attacked, we will allow them in, they'd just have to
// be patient (max 20 seconds).
usleep($waitMs * 4000);
}

$doUser = OA_Dal::factoryDO('users');
$doUser->username = strtolower($username);
$doUser->username = $username;
$doUser->password = md5($password);

$doUser->find();

if ($doUser->fetch()) {
$oLock->release();

return $doUser;
} else {
return false;
}

if ($lock) {
// The password was wrong, but no other login attempt was in place
// so we apply just the base delay time.
usleep($waitMs * 1000);
}

$oLock->release();

return false;
}

/**
Expand Down Expand Up @@ -517,4 +557,7 @@ function validateUsersData($data)
}
}

?>

class Plugins_Authentication_Exception extends Exception
{
}

0 comments on commit 8479413

Please sign in to comment.