Vulnerability: $_SESSION spoofing can give superadmin access

[brainexcerpts]

I was working on two different instances of PivotX on a local server (Xampp).
I discovered that, login on one instance, would allow me to get access to the other instance's administration panel...
By instance I mean: two copies of PivotX in different folders placed in /htdocs.
Even when the two versions have distinct user names, passwords etc.
It allows some level of accessibility in the unrelated instance.
Worse, using the same username gives you the same level of credential in the other instance.

Multisites scenarios on the same web server might be rare, but I think it is still a security issue.
It's also a bug since user sessions of different instances interferes with each other.

Cause

It turns out the function isLoggedIn() blindly trust $_SESSION:

function isLoggedIn() {
    global $PIVOTX;
    $this->loadPermsessions();
    $sessioncookie = (!empty($_COOKIE['pivotxsession'])) ? $_COOKIE['pivotxsession'] : $_POST['pivotxsession'];
    if (isset($_SESSION['user']) && isset($_SESSION['user']['username']) && ($_SESSION['user']['username']!="") ) {
        // User is logged in!
        // Check if we're in the saved sessions..
        if (!empty($sessioncookie) && !isset($this->permsessions[$sessioncookie])) {
    
            $this->permsessions[ $sessioncookie ] = array(
                'username' => $_SESSION['user']['username'],
                'ip' => $_SERVER['REMOTE_ADDR'],
                'lastseen' => time()
            );
            $this->savePermsessions();
        }
        return true;
    } else {
         ...
        skipping stuff
        ...
        return false;
    }
}

It seems that $_SESSION['user']['username'] only needs to be set to some value for pivotx to view the user as "logged in"
(and this regardless of its actual value or checking anything else).

If the attacker finds out the username of the superadmin,
(for instance when the nickname is the same as the user name and displayed in a blog post),
he can get full privileges on the other instance without knowing its password.

If one finds a way to set $_SESSION['user']['username'] by somehow executing some lines of php, another whole PivotX instance is not even necessary (but I'll admit that in this case you are doomed anyway 😅 ).

Aside from the security issue, it also means you have to log out of one instance before using the other instance.
Otherwise the two instances will interfere with each other by login you with the user privileges of the instance you have just used.
You will end up with the wrong user name and settings to publish. So this behavior is also a bug in itself.

Fix

My proposed fix involves isLoggedIn() and function login($username, $password, $stay) we make sure to check $_SESSION user name actually exists in the db, and that the stored password actually matches the $_SESSION password.
This version will log you out if you change your password, which makes more sens as well.
Now we can login on two different instances without conflicts even when the usernames are exactly the same.

I did not push it as far as comparing the settings of the two users to be exactly the same,
from a security standpoint that would be useless, however,
someone using various instances may use the same passwords and username but different settings (pretty niche feature maybe).

// fix and refactoring of 'if' imbrications to avoid unnecessary indentation.
function isLoggedIn() {
    global $PIVOTX;

    $this->loadPermsessions();

    $sessioncookie = (!empty($_COOKIE['pivotxsession'])) ? $_COOKIE['pivotxsession'] : $_POST['pivotxsession'];

    $isTrustedSession = false;
    $hasSession = isset($_SESSION['user']) && isset($_SESSION['user']['username']) && ($_SESSION['user']['username']!="");
    if ( $hasSession ) {
        if ( $PIVOTX['users']->isUser($_SESSION['user']['username']) ) {
            $userInfos = $PIVOTX['users']->getUser($_SESSION['user']['username']);
            if( $userInfos['password'] == $_SESSION['user']['password'] ) {
                $isTrustedSession = true;
            }
        }
    }

    if ( $isTrustedSession )
    {
        // Check if we're in the saved sessions..
        if (!empty($sessioncookie) && !isset($this->permsessions[$sessioncookie])) {
            $this->permsessions[ $sessioncookie ] = array(
                'username' => $_SESSION['user']['username'],
                'ip' => $_SERVER['REMOTE_ADDR'],
                'lastseen' => time(),
                'password' => $_SESSION['user']['password'] // we can trust this password
            );
            $this->savePermsessions();
        }

        return true;
    }

    // See if we can continue a stored session..
    // Check if we have a pivotxsession cookie that matches a saved session..
    if ( empty($sessioncookie) || !isset($this->permsessions[$sessioncookie]) ) {
        return false;
    }

    $savedsess = $this->permsessions[ $sessioncookie ];
    // Check if the IP in the saved session matches the IP of the user..
    if ($_SERVER['REMOTE_ADDR'] != $savedsess['ip']) {
        return false;
    }

    // Check if the 'lastseen' wasn't too long ago..
    if (time() >= ($savedsess['lastseen'] + $this->cookie_lifespan) ) {
        return false;
    }

    // Finally check if the user in the stored session still exists.
    if (!$PIVOTX['users']->isUser($savedsess['username'])) {
        return false;
    }

    // check password
    $userInfos = $PIVOTX['users']->getUser($savedsess['username']);

    // In the off chance that an attacker has managed to aquire the cookie,
    // we still need to check the password.
    // changed password should invalidate the session anyway.
    if( $userInfos['password'] != $savedsess['password'] ) {
        return false;
    }

    // If we get here, we can restore the session!

    $_SESSION['user'] = $userInfos;

    // Update the 'lastseen' in permanent sessions.
    $this->permsessions[ $sessioncookie ]['lastseen'] = time();
    $this->savePermsessions();

    // Add the 'lastseen' to the user settings.
    $PIVOTX['users']->updateUser($savedsess['username'], array('lastseen'=>time()) );
    $_SESSION['user']['lastseen'] = time();

    // Set the session cookie as session variable.
    $_SESSION['pivotxsession'] = $sessioncookie;

    return true;
}

function login($username, $password, $stay) { 
     ...
     ...
    // Only the following needs to be patched:
    // we add the hashed password in the session file for future checks with $_SESSION:
    $this->permsessions[ $key ] = array(
                    'username' => $username,
                    'ip' => $_SERVER['REMOTE_ADDR'],
                    'lastseen' => time(),
                    'password' => $_SESSION['user']['password']
                );
    ...
    ...
}

[hansfn]

Thanks for the report. I haven't looked closely at it yet, but I just wanted to let you know that I have noticed it. I will respond in a few days. (When providing a fix, you should supply a patch or PR with minimal changes making the review easier. Other improvements should be a separate patch / PR.)

Multisites scenarios on the same web server might be rare, but I think it is still a security issue.
It's also a bug since user sessions of different instances interferes with each other.

I totally agree it is an issue.

PS! I would very much have preferred if you reported this to me / us privately so we could have a fix ready before you made the issue public. (In other words, we prefer private or responsible disclosure over full disclosure.)

[brainexcerpts]

Thank you for the swift response.

Sorry about the full disclosure 🙇 It's actually the first time I report any kind of vulnerability anywhere, I didn't really thought this through. Feel free to delete/hide the ticket if you feel that's safer.

To be honest, I was uncertain if you'd actually prefer a PR over a ticket that you'd implement / oversight yourself. I can make a PR once my other PR on flat file db corruption gets reviewed.

[hansfn]

First, after reading this issue again, I think it's a non-issue:

• In general: If you create a valid session on the web server using some PHP code, then you can for example just create some code that overwrite the PivotX user database and change the password. There is no need to spoof the session. (My recovery tool did just that - rewrote the user database.)
• Then you have the situation with multiple PivotX instances on the same server / domain. This is normally not a problem since they should control different parts of the website. See below for more information. You only have the problem you describe if both instances publish to the same directory. That is a very bad setup ...

It turns out the function isLoggedIn() blindly trust $_SESSION

Yes, and in most situations it should. Let me fist show you how this works and then we can discuss how and if we should tighten the security.

In the Session class (in objects.php) we set various properties for the session cookie using session_set_cookie_params - domain, path and so on. On line 2011 of objects.php we have:

    // Set to 'site url' instead of 'pivotx_url', because then we
    // can use 'edit this entry' and the like.
    $this->cookie_path = $PIVOTX['paths']['site_url']; 

If we use $PIVOTX['paths']['pivotx_url'], it is 100% safe to "blindly trust" the session. However, for convenience we decided to use $PIVOTX['paths']['site_url']. That might cause problems in very special situations.

So, I assume you tested the following setup:

example.org/sub1/pivotx
example.org/sub2/pivotx

where both PivotX installations had site URL set to '/' - meaning the session cookie lived at the root and hence both installations could see each others sessions.

Final remarks

I haven't had time to test if my description above is correct. I have just read the code. I would appreciate if you verified my findings by modifying the site URL and so on.

And, I would prefer to continue to use $PIVOTX['paths']['site_url'] as cookie path, and rather document that having multiples PivotX installations on the same domain is fragile - you can overwrite each others content and maybe even log in as a superadmin if the usernames match.

PS! As you explain: A superadmin on one site can create a user with matching name, and spoof the session for a superadmin on the other site.