The logout endpoint provided by the Shibboleth plugin for an Identity Provider to log a user out of services had an odd way of finding the right sessions to destroy that lead to the request originator being logged into another seemingly random account.

foreach ($sessions as $session) {
    if (session_decode(base64_decode($session->sessdata))) {
        if ($_SESSION['SESSION']->shibboleth_session_id == trim($spsessionid)) {
            // ...
        }
    }
}

This happened when a database was used as the backing storage for sessions. The application would query the database for all sessions, then it would session_decode each of them (which deserializes the value into the $_SESSION superglobal) and check if the id matched the id they were trying to logout.

The problem is two fold, first, for a brief time the user making the request would have their session set to the session of every active user-session, though exploiting anything in this short window might not be practical. Secondly and where the main session hijack comes in is that after the loop ended this process wasn’t reversed. The user would be logged in as the last session in the response. If it wasn’t a privileged user, one could use the main logout (this is one used by the SAML idP) to have the session destroyed and try again until they got an admin account.

Great find by Robin Peraglie and Johannes Moritz, fun bug, but stupid code on Moodle’s part. Though PHP can take a bit of the blame as while the language exposes unserialize this doesn’t work on the session data without a bit of extra parsing (which is ultimately what the patch does). So I understand why developers might be tempted to do it this way.