From 1bb91383b585f46d8754af714e98c6549aa3c47a Mon Sep 17 00:00:00 2001
From: Jaime Perez Crespo <jaime.perez@uninett.no>
Date: Thu, 7 Apr 2016 16:22:47 +0200
Subject: [PATCH] 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.

---
 lib/SimpleSAML/SessionHandlerPHP.php | 53 ++++++++++++++--------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/lib/SimpleSAML/SessionHandlerPHP.php b/lib/SimpleSAML/SessionHandlerPHP.php
index da031ebe1..c72b0a45f 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();
     }
-- 
GitLab