CVE-2016-9125: Session Fixation (CWE-384)
value: Revive Adserver before 3.2.3 suffers from session fixation, by allowing arbitrary session identifiers to be forced and, at the same time, by not invalidating the existing session upon a successful authentication. Under some circumstances, that could have been an opportunity for an attacker to steal an authenticated session.
CWE description: Authenticating a user, or otherwise establishing a new user session, without invalidating any existing session identifier gives an attacker the opportunity to steal authenticated sessions.
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/OA/Upgrade/Login.php, lib/max/Dal/Admin/Session.php, www/admin/lib-sessions.inc.php, www/api/v1/xmlrpc/LogonServiceImpl.php, www/api/v2/xmlrpc/LogonServiceImpl.php
Commit date: 2016-03-01 16:23:09 +0100
Commit message: Fix h1 reports 93809 and 93813

Session Fixation
----------------

The HackerOne users kaviya and Kamini Singh have independendltly reported
that Revive Adserver was vulnerable to session fixation, by allowing arbitrary
session identifiers to be forced and at the same time by not invalidating the
existing session upon a successful authentication. Under some circumstances,
that could have been an opportunity for an attacker to steal an authenticated
sessions.

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

CWE: CWE-384
CVSSv2: 7.8 (AV:N/AC:M/Au:N/C:C/I:P/A:N)

Diff: diff --git a/lib/OA/Auth.php b/lib/OA/Auth.php
index c8b7bfac39..3a0c8af0de 100644
--- a/lib/OA/Auth.php
+++ b/lib/OA/Auth.php
@@ -84,10 +84,13 @@ class OA_Auth
 
             if (!$doUser) {
                 OA_Auth::restart($GLOBALS['strUsernameOrPasswordWrong']);
             }
 
+            // Regenerate session ID now
+            phpAds_SessionRegenerateId();
+
             return OA_Auth::getSessionData($doUser);
         }
 
         OA_Auth::restart();
     }
@@ -188,11 +191,11 @@ class OA_Auth
      *
      * @param string $sMessage Optional message
      */
     function restart($sMessage = '')
     {
-        $_COOKIE['sessionID'] = phpAds_SessionStart();
+        $_COOKIE['sessionID'] = phpAds_SessionRegenerateId();
         OA_Auth::displayLogin($sMessage, $_COOKIE['sessionID']);
     }
 
     /**
      * A static method to restart with a login screen, displaying an error message
diff --git a/lib/OA/Upgrade/Login.php b/lib/OA/Upgrade/Login.php
index 7b73f0f90f..5d319a39d8 100644
--- a/lib/OA/Upgrade/Login.php
+++ b/lib/OA/Upgrade/Login.php
@@ -23,11 +23,11 @@ class OA_Upgrade_Login
      * @return boolean True if login succeded
      */
     function checkLogin()
     {
         // Clean up session
-        $GLOBALS['session'] = array();
+        phpAds_clearSession();
 
         // Detection needs to happen every time to make sure that database parameters are
         $oUpgrader = new OA_Upgrade();
         $openadsDetected = $oUpgrader->detectOpenads(true) ||
             $oUpgrader->existing_installation_status == OA_STATUS_CURRENT_VERSION;
@@ -91,10 +91,11 @@ class OA_Upgrade_Login
             $doAUA = OA_Dal::factoryDO('account_user_assoc');
             $doAUA->account_id = $adminAccountId;
             $doUser->joinAdd($doAUA);
             $doUser->find();
             if ($doUser->fetch()) {
+                phpAds_SessionRegenerateId();
                 phpAds_SessionDataRegister(OA_Auth::getSessionData($doUser));
                 phpAds_SessionDataStore();
             }
         }
     }
@@ -104,10 +105,12 @@ class OA_Upgrade_Login
         $oPlugin = new Plugins_Authentication();
 
         $aCredentials = $oPlugin->_getCredentials(false);
 
         if (!PEAR::isError($aCredentials)) {
+            phpAds_SessionRegenerateId();
+
             $doUser = $oPlugin->checkPassword($aCredentials['username'], $aCredentials['password']);
 
             if ($doUser) {
                 phpAds_SessionDataRegister(OA_Auth::getSessionData($doUser));
             }
@@ -138,10 +141,12 @@ class OA_Upgrade_Login
             if (is_array($aPref)) {
                 $oPlugin = OA_Auth::staticGetAuthPlugin('internal');
                 $aCredentials = $oPlugin->_getCredentials(false);
 
                 if (!PEAR::isError($aCredentials)) {
+                    phpAds_SessionRegenerateId();
+
                     if (strtolower($aPref['admin']) == strtolower($aCredentials['username']) &&
                         $aPref['admin_pw'] == md5($aCredentials['password']))
                     {
                         $doUser = OA_Dal::factoryDO('users');
                         $doUser->username = $aPref['admin'];
diff --git a/lib/max/Dal/Admin/Session.php b/lib/max/Dal/Admin/Session.php
index 1e917c7f34..8472efd940 100644
--- a/lib/max/Dal/Admin/Session.php
+++ b/lib/max/Dal/Admin/Session.php
@@ -74,14 +74,14 @@ class MAX_Dal_Admin_Session extends MAX_Dal_Common
     {
         $doSession = OA_Dal::staticGetDO('session', $session_id);
         if ($doSession) {
             $doSession->sessiondata = $serialized_session_data;
             $doSession->update();
-        }
-        else {
+        } else {
             $doSession = OA_Dal::factoryDO('session');
-            $doSession->sessionid = $session_id;
+            // It's an md5, so 32 chars max
+            $doSession->sessionid = substr($session_id, 0, 32);
             $doSession->sessiondata = $serialized_session_data;
             $doSession->insert();
         }
     }
 
diff --git a/www/admin/lib-sessions.inc.php b/www/admin/lib-sessions.inc.php
index 689421f9af..3a304708aa 100644
--- a/www/admin/lib-sessions.inc.php
+++ b/www/admin/lib-sessions.inc.php
@@ -28,32 +28,33 @@ require_once MAX_PATH . '/lib/max/Dal/Admin/Session.php';
  * @todo Move to a domain-layer class library.
  */
 function phpAds_SessionDataFetch()
 {
     global $session;
-    $dal = new MAX_Dal_Admin_Session();
 
     // Guard clause: Can't fetch a session without an ID
-	if (empty($_COOKIE['sessionID'])) {
+    if (empty($_COOKIE['sessionID']) || !preg_match('#^[0-9a-f]{32}$#D', $_COOKIE['sessionID'])) {
         return;
     }
 
+    $dal = new MAX_Dal_Admin_Session();
     $serialized_session = $dal->getSerializedSession($_COOKIE['sessionID']);
 
-    // This is required because 'sessionID' cookie is set to new during logout.
-    // According to comments in the file it is because some servers do not
-    // support setting cookies during redirect.
-    if (empty($serialized_session)) {
+    // Return if the session was not found (expired or forged)
+    if (!$serialized_session) {
         return;
     }
 
     $loaded_session = unserialize($serialized_session);
-	if (!$loaded_session) {
-        // XXX: Consider raising an error
+
+    // Or if it can't be unserialized and/or is not a session we started
+    if (empty($loaded_session['__authentic__'])) {
         return;
     }
-	$session = $loaded_session;
+
+    $session = $loaded_session;
+
     $dal->refreshSession($_COOKIE['sessionID']);
 }
 
 /*-------------------------------------------------------*/
 /* Sets an admin cookie                                  */
@@ -73,25 +74,79 @@ function phpAds_SessionSetAdminCookie($name, $value)
         true
     );
 }
 
 /*-------------------------------------------------------*/
-/* Create a new sessionid                                */
+/* Start a new session                                   */
 /*-------------------------------------------------------*/
 
 function phpAds_SessionStart()
 {
 	global $session;
+
 	if (empty($_COOKIE['sessionID'])) {
-		$session = array();
-		$_COOKIE['sessionID'] = md5(uniqid('phpads', 1));
+		phpAds_clearSession();
+
+        $sessionId = phpAds_SessionGenerateId();
 
-        phpAds_SessionSetAdminCookie('sessionID', $_COOKIE['sessionID']);
+        $dal = new MAX_Dal_Admin_Session();
+        $dal->storeSerializedSession(serialize($session), $sessionId);
 	}
+
 	return $_COOKIE['sessionID'];
 }
 
+/*-------------------------------------------------------*/
+/* Generate a sessionid                                  */
+/*-------------------------------------------------------*/
+
+function phpAds_SessionGenerateId()
+{
+    $_COOKIE['sessionID'] = md5(uniqid('phpads', 1));
+
+    phpAds_SessionSetAdminCookie('sessionID', $_COOKIE['sessionID']);
+
+    return $_COOKIE['sessionID'];
+}
+
+/*-------------------------------------------------------*/
+/* Re-generate the sessionid                             */
+/*-------------------------------------------------------*/
+
+function phpAds_SessionRegenerateId()
+{
+    global $session;
+
+    $dal = new MAX_Dal_Admin_Session();
+
+    if (!empty($_COOKIE['sessionID'])) {
+        $dal->deleteSession($_COOKIE['sessionID']);
+    }
+
+    if (!empty($session['__authentic__'])) {
+        $sessionId = phpAds_SessionGenerateId();
+        $dal->storeSerializedSession(serialize($session), $sessionId);
+
+        return $sessionId;
+    }
+
+    unset($_COOKIE['sessionID']);
+
+    return phpAds_SessionStart();
+}
+
+/*-------------------------------------------------------*/
+/* Clear the session and mark it as authentic            */
+/*-------------------------------------------------------*/
+
+function phpAds_clearSession()
+{
+    $GLOBALS['session'] = array(
+        '__authentic__' => true,
+    );
+}
+
 /*-------------------------------------------------------*/
 /* Register the data in the session array                */
 /*-------------------------------------------------------*/
 
 function phpAds_SessionDataRegister($key, $value='')
diff --git a/www/api/v1/xmlrpc/LogonServiceImpl.php b/www/api/v1/xmlrpc/LogonServiceImpl.php
index acececac6c..8781e33c28 100644
--- a/www/api/v1/xmlrpc/LogonServiceImpl.php
+++ b/www/api/v1/xmlrpc/LogonServiceImpl.php
@@ -91,11 +91,12 @@ class LogonServiceImpl extends BaseServiceImpl
         $_POST['username'] = $username;
         $_POST['password'] = $password;
 
         $_POST['login'] = 'Login';
 
-        $_COOKIE['sessionID'] = uniqid('phpads', 1);
+        unset($_COOKIE['sessionID']);
+        phpAds_SessionStart();
         $_POST['phpAds_cookiecheck'] = $_COOKIE['sessionID'];
 
         $this->preInitSession();
         if ($this->_internalLogin($username, $password)) {
             // Check if the user has administrator access to Openads.
diff --git a/www/api/v2/xmlrpc/LogonServiceImpl.php b/www/api/v2/xmlrpc/LogonServiceImpl.php
index ef617ea8fe..deb0db11c5 100644
--- a/www/api/v2/xmlrpc/LogonServiceImpl.php
+++ b/www/api/v2/xmlrpc/LogonServiceImpl.php
@@ -91,11 +91,12 @@ class LogonServiceImpl extends BaseServiceImpl
         $_POST['username'] = $username;
         $_POST['password'] = $password;
 
         $_POST['login'] = 'Login';
 
-        $_COOKIE['sessionID'] = uniqid('phpads', 1);
+        unset($_COOKIE['sessionID']);
+        phpAds_SessionStart();
         $_POST['phpAds_cookiecheck'] = $_COOKIE['sessionID'];
 
         $this->preInitSession();
         if ($this->_internalLogin($username, $password)) {
             // Check if the user has administrator access to Openads.

Vulnerable code:         $_COOKIE['sessionID'] = phpAds_SessionStart();
        $GLOBALS['session'] = array();
        }
        else {
            $doSession->sessionid = $session_id;
    $dal = new MAX_Dal_Admin_Session();
	if (empty($_COOKIE['sessionID'])) {
    // This is required because 'sessionID' cookie is set to new during logout.
    // According to comments in the file it is because some servers do not
    // support setting cookies during redirect.
    if (empty($serialized_session)) {
	if (!$loaded_session) {
        // XXX: Consider raising an error
	$session = $loaded_session;
/* Create a new sessionid                                */
		$session = array();
		$_COOKIE['sessionID'] = md5(uniqid('phpads', 1));
        phpAds_SessionSetAdminCookie('sessionID', $_COOKIE['sessionID']);
        $_COOKIE['sessionID'] = uniqid('phpads', 1);
        $_COOKIE['sessionID'] = uniqid('phpads', 1);
Fixed code:             // Regenerate session ID now
            phpAds_SessionRegenerateId();

        $_COOKIE['sessionID'] = phpAds_SessionRegenerateId();
        phpAds_clearSession();
                phpAds_SessionRegenerateId();
            phpAds_SessionRegenerateId();

                    phpAds_SessionRegenerateId();

        } else {
            // It's an md5, so 32 chars max
            $doSession->sessionid = substr($session_id, 0, 32);
    if (empty($_COOKIE['sessionID']) || !preg_match('#^[0-9a-f]{32}$#D', $_COOKIE['sessionID'])) {
    $dal = new MAX_Dal_Admin_Session();
    // Return if the session was not found (expired or forged)
    if (!$serialized_session) {

    // Or if it can't be unserialized and/or is not a session we started
    if (empty($loaded_session['__authentic__'])) {

    $session = $loaded_session;

/* Start a new session                                   */

		phpAds_clearSession();

        $sessionId = phpAds_SessionGenerateId();
        $dal = new MAX_Dal_Admin_Session();
        $dal->storeSerializedSession(serialize($session), $sessionId);

/*-------------------------------------------------------*/
/* Generate a sessionid                                  */
/*-------------------------------------------------------*/

function phpAds_SessionGenerateId()
{
    $_COOKIE['sessionID'] = md5(uniqid('phpads', 1));

    phpAds_SessionSetAdminCookie('sessionID', $_COOKIE['sessionID']);

    return $_COOKIE['sessionID'];
}

/*-------------------------------------------------------*/
/* Re-generate the sessionid                             */
/*-------------------------------------------------------*/

function phpAds_SessionRegenerateId()
{
    global $session;

    $dal = new MAX_Dal_Admin_Session();

    if (!empty($_COOKIE['sessionID'])) {
        $dal->deleteSession($_COOKIE['sessionID']);
    }

    if (!empty($session['__authentic__'])) {
        $sessionId = phpAds_SessionGenerateId();
        $dal->storeSerializedSession(serialize($session), $sessionId);

        return $sessionId;
    }

    unset($_COOKIE['sessionID']);

    return phpAds_SessionStart();
}

/*-------------------------------------------------------*/
/* Clear the session and mark it as authentic            */
/*-------------------------------------------------------*/

function phpAds_clearSession()
{
    $GLOBALS['session'] = array(
        '__authentic__' => true,
    );
}

        unset($_COOKIE['sessionID']);
        phpAds_SessionStart();
        unset($_COOKIE['sessionID']);
        phpAds_SessionStart();