Bugfix: when using PHP sessions, if there's already a session, session_id() will return the identifier of that session, not our session. In that case, we need to make sure it is our session so that we don't hijack the one of the application.

1 files changed, 27 insertions, 26 deletions

diff --git a/lib/SimpleSAML/SessionHandlerPHP.php b/lib/SimpleSAML/SessionHandlerPHP.php
index da031eb..c72b0a4 100644
--- a/lib/SimpleSAML/SessionHandlerPHP.php
+++ b/lib/SimpleSAML/SessionHandlerPHP.php
@@ -107,36 +107,37 @@ class SimpleSAML_SessionHandlerPHP extends SimpleSAML_SessionHandler
      */
     public function getCookieSessionId()
     {
-        if (session_id() === '') {
-            if (!self::hasSessionCookie()) {
-                return null;
-            }
+        if (!self::hasSessionCookie()) {
+            return null; // there's no session cookie, can't return ID
+        }
 
-            $session_cookie_params = session_get_cookie_params();
+        // do not rely on session_id() as it can return the ID of a previous session. Get it from the cookie instead.
+        session_id($_COOKIE[$this->cookie_name]);
 
-            if ($session_cookie_params['secure'] && !\SimpleSAML\Utils\HTTP::isHTTPS()) {
-                throw new SimpleSAML_Error_Exception('Session start with secure cookie not allowed on http.');
-            }
+        $session_cookie_params = session_get_cookie_params();
 
-            $cacheLimiter = session_cache_limiter();
-            if (headers_sent()) {
-                /*
-                 * session_start() tries to send HTTP headers depending on the configuration, according to the
-                 * documentation:
-                 *
-                 *      http://php.net/manual/en/function.session-start.php
-                 *
-                 * If headers have been already sent, it will then trigger an error since no more headers can be sent.
-                 * Being unable to send headers does not mean we cannot recover the session by calling session_start(),
-                 * so we still want to call it. In this case, though, we want to avoid session_start() to send any
-                 * headers at all so that no error is generated, so we clear the cache limiter temporarily (no headers
-                 * sent then) and restore it after successfully starting the session.
-                 */
-                session_cache_limiter('');
-            }
-            session_start();
-            session_cache_limiter($cacheLimiter);
+        if ($session_cookie_params['secure'] && !\SimpleSAML\Utils\HTTP::isHTTPS()) {
+            throw new SimpleSAML_Error_Exception('Session start with secure cookie not allowed on http.');
+        }
+
+        $cacheLimiter = session_cache_limiter();
+        if (headers_sent()) {
+            /*
+             * session_start() tries to send HTTP headers depending on the configuration, according to the
+             * documentation:
+             *
+             *      http://php.net/manual/en/function.session-start.php
+             *
+             * If headers have been already sent, it will then trigger an error since no more headers can be sent.
+             * Being unable to send headers does not mean we cannot recover the session by calling session_start(),
+             * so we still want to call it. In this case, though, we want to avoid session_start() to send any
+             * headers at all so that no error is generated, so we clear the cache limiter temporarily (no headers
+             * sent then) and restore it after successfully starting the session.
+             */
+            session_cache_limiter('');
         }
+        session_start();
+        session_cache_limiter($cacheLimiter);
 
         return session_id();
     }