From f8ee2195e92a4ecf1a6c8eafdd8ea6ae41ddf434 Mon Sep 17 00:00:00 2001
From: Jaime Perez Crespo <jaime.perez@uninett.no>
Date: Thu, 7 Apr 2016 16:33:49 +0200
Subject: [PATCH] Bugfixes: the PHP session handler only fetches the cookie
 configuration parameters if session_id() returns a non-empty ID. This won't
 happen if the application initialized a session previously, hijacking the
 session and causing all kinds of trouble. Instead, we need to detect if
 there's an active session, save its parameters and close it. After closing
 it, we can name a new session and set the cookie parameters.

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

diff --git a/lib/SimpleSAML/SessionHandlerPHP.php b/lib/SimpleSAML/SessionHandlerPHP.php
index c72b0a45f..9126eab27 100644
--- a/lib/SimpleSAML/SessionHandlerPHP.php
+++ b/lib/SimpleSAML/SessionHandlerPHP.php
@@ -19,6 +19,18 @@ class SimpleSAML_SessionHandlerPHP extends SimpleSAML_SessionHandler
      */
     protected $cookie_name;
 
+    /**
+     * An associative array containing the details of a session existing previously to creating or loading one with this
+     * session handler. The keys of the array will be:
+     *
+     *   - id: the ID of the session, as returned by session_id().
+     *   - name: the name of the session, as returned by session_name().
+     *   - cookie_params: the parameters of the session cookie, as returned by session_get_cookie_params().
+     *
+     * @var array
+     */
+    private $previous_session = array();
+
 
     /**
      * Initialize the PHP session handling. This constructor is protected because it should only be called from
@@ -29,35 +41,41 @@ class SimpleSAML_SessionHandlerPHP extends SimpleSAML_SessionHandler
         // call the parent constructor in case it should become necessary in the future
         parent::__construct();
 
-        /* Initialize the php session handling.
-         *
-         * If session_id() returns a blank string, then we need to call session start. Otherwise the session is already
-         * started, and we should avoid calling session_start().
-         */
-        if (session_id() === '') {
-            $config = SimpleSAML_Configuration::getInstance();
-
-            $params = $this->getCookieParams();
-
-            session_set_cookie_params(
-                $params['lifetime'],
-                $params['path'],
-                $params['domain'],
-                $params['secure'],
-                $params['httponly']
-            );
+        if (session_status() === PHP_SESSION_ACTIVE) {
+            /*
+             * We shouldn't have a session at this point, so it might be an application session. Save the details to
+             * retrieve it later and commit.
+             */
+            $this->previous_session['cookie_params'] = session_get_cookie_params();
+            $this->previous_session['id'] = session_id();
+            $this->previous_session['name'] = session_name();
+            session_write_close();
+        }
+
+        $config = SimpleSAML_Configuration::getInstance();
+
+        $this->cookie_name = $config->getString('session.phpsession.cookiename', null);
+        if (!empty($this->cookie_name)) {
+            session_name($this->cookie_name);
+        } else {
+            $this->cookie_name = session_name();
+        }
+
+        $params = $this->getCookieParams();
+
+        session_set_cookie_params(
+            $params['lifetime'],
+            $params['path'],
+            $params['domain'],
+            $params['secure'],
+            $params['httponly']
+        );
+
+        $savepath = $config->getString('session.phpsession.savepath', null);
+        if (!empty($savepath)) {
+            session_save_path($savepath);
 
-            $this->cookie_name = $config->getString('session.phpsession.cookiename', null);
-            if (!empty($this->cookie_name)) {
-                session_name($this->cookie_name);
-            } else {
-                $this->cookie_name = session_name();
-            }
 
-            $savepath = $config->getString('session.phpsession.savepath', null);
-            if (!empty($savepath)) {
-                session_save_path($savepath);
-            }
         }
     }
 
-- 
GitLab