From 2b414f75b62efe80cb89348b68df2d1ee430b908 Mon Sep 17 00:00:00 2001
From: Carsten Rose <carsten.rose@math.uzh.ch>
Date: Wed, 14 Mar 2018 22:44:02 +0100
Subject: [PATCH] Bug #5668 / Broken SIP after login. Session.php: After
setting the QFQ cookie lifetime AND calling setcookie() after
session_start(), the QFQ session remains. Also, skipping Session::clearAll()
protects against broken SIP after logout!
---
extension/qfq/qfq/store/Session.php | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/extension/qfq/qfq/store/Session.php b/extension/qfq/qfq/store/Session.php
index c0ef096bd..45a0f3c75 100644
--- a/extension/qfq/qfq/store/Session.php
+++ b/extension/qfq/qfq/store/Session.php
@@ -23,23 +23,31 @@ class Session {
* @throws CodeException
*/
private function __construct($phpUnit = false) {
+
if (self::$phpUnit !== null) {
throw new CodeException("Try to set flag phpunit again - that should not happen.", ERROR_CODE_SHOULD_NOT_HAPPEN);
}
self::$phpUnit = $phpUnit;
+
if (self::$phpUnit === true) {
self::$sessionLocal = array();
} else {
ini_set('session.cookie_httponly', 1);
+ $lifetime = 86400; // one day
+
$path = $this->getSitePath();
- session_set_cookie_params(0, $path);
+ session_set_cookie_params($lifetime, $path);
+ $currentCookieParams = session_get_cookie_params();
session_name(SESSION_NAME);
session_start();
+ // Currently, setcookie() is only called to really extend the lifetime. All other parameter needs to be given again.
+ setcookie(SESSION_NAME, session_id(), time() + $lifetime, $path, $currentCookieParams['domain'], $currentCookieParams['secure'], true);
+
self::$sessionId = session_id();
}
@@ -62,6 +70,7 @@ class Session {
$path = $_SERVER['SCRIPT_NAME'];
$pos = strrpos($path, '/');
+
if ($pos === false) {
throw new CodeException("Broken _SERVER[SCRIPT_NAME]: $path", ERROR_SESSION_BROKEN_SCRIPT_PATH);
}
@@ -97,6 +106,7 @@ class Session {
* Destroy a session - this is only needed in case of attacks
*/
public static function destroy() {
+
session_destroy();
$_SESSION = array();
@@ -116,6 +126,7 @@ class Session {
*
*/
public static function open() {
+
if (self::$sessionOpen != true && self::$sessionId != null) {
session_id(self::$sessionId);
session_start();
@@ -148,9 +159,9 @@ class Session {
$feUidLoggedIn = $feUserUidSession;
}
- if ($feUidLoggedIn !== $feUserUidSession) {
+ if ($feUidLoggedIn != $feUserUidSession) {
// destroy existing session store
- Session::clearAll();
+// Session::clearAll(); // #5668 / Broken SIP after login - is it really a security improvement to destroy the SIP store in case the feUser changes? Probably not.
// save new feUserUid, feUserName
Session::set(SESSION_FE_USER_UID, $feUidLoggedIn);
--
GitLab