CVE-2016-9124: Improper Authentication (CWE-287)
value: Revive Adserver before 3.2.3 suffers from Improper Restriction of Excessive Authentication Attempts. The login page of Revive Adserver is vulnerable to password-guessing attacks. An account lockdown feature was considered, but rejected to avoid introducing service disruptions to regular users during such attacks. A random delay has instead been introduced as a countermeasure in case of password failures, along with a system to discourage parallel brute forcing. These systems will effectively allow the valid users to log in to the adserver, even while an attack is in progress.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 9.8
Published: 2017-03-28T02:59Z
Repository: https://github.com/revive-adserver/revive-adserver
Language: PHP
Files changed: lib/OA/Auth.php, lib/OX/Extension/authentication/authentication.php
Commit date: 2016-02-10 17:54:48 +0100
Commit message: 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)

Diff: diff --git a/lib/OA/Auth.php b/lib/OA/Auth.php
index f5dec0ba0..c8b7bfac3 100644
--- a/lib/OA/Auth.php
+++ b/lib/OA/Auth.php
@@ -81,11 +81,10 @@ class OA_Auth
 
         if (OA_Auth::suppliedCredentials()) {
             $doUser = OA_Auth::authenticateUser();
 
             if (!$doUser) {
-                sleep(3);
                 OA_Auth::restart($GLOBALS['strUsernameOrPasswordWrong']);
             }
 
             return OA_Auth::getSessionData($doUser);
         }
diff --git a/lib/OX/Extension/authentication/authentication.php b/lib/OX/Extension/authentication/authentication.php
index 7b4153e12..f9dd02062 100644
--- a/lib/OX/Extension/authentication/authentication.php
+++ b/lib/OX/Extension/authentication/authentication.php
@@ -13,10 +13,11 @@
 require_once MAX_PATH . '/lib/max/Plugin/Translation.php';
 require_once LIB_PATH . '/Plugin/Component.php';
 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');
 
 /**
  * Plugins_Authentication is an parent class for Authentication plugins
@@ -96,20 +97,59 @@ class Plugins_Authentication extends OX_Component
      * @param string $password
      * @return mixed A DataObjects_Users instance, or false if no matching user was found
      */
     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;
     }
 
     /**
      * Cleans up the session and carry on any additional tasks required to logout the user
      *
@@ -515,6 +555,9 @@ class Plugins_Authentication extends OX_Component
 
         return $this->getValidationErrors();
     }
 }
 
-?>
\ No newline at end of file
+
+class Plugins_Authentication_Exception extends Exception
+{
+}

Vulnerable code:                 sleep(3);
        $doUser->username = strtolower($username);
        } else {
            return false;
?>
Fixed code: require_once MAX_PATH . '/lib/OA/DB/AdvisoryLock.php';
        // 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->username = $username;

            $oLock->release();


        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;

class Plugins_Authentication_Exception extends Exception
{
}