From 281ac3625864eb5f037395376b4b40fe69f789da Mon Sep 17 00:00:00 2001
From: =?utf8?q?Tim=20D=C3=BCsterhus?= <duesterhus@woltlab.com>
Date: Tue, 9 Dec 2014 21:42:46 +0100
Subject: [PATCH] Properly prevent session fixation in every case

---
 com.woltlab.wcf/update_2.1.0_alpha_1.sql      |  4 +-
 .../editor/MySQLDatabaseEditor.class.php      |  2 +
 .../editor/PostgreSQLDatabaseEditor.class.php |  2 +
 .../system/database/util/SQLParser.class.php  | 10 ++-
 .../system/session/SessionHandler.class.php   | 74 +++++++++++--------
 wcfsetup/setup/db/install.sql                 |  2 +-
 6 files changed, 58 insertions(+), 36 deletions(-)

diff --git a/com.woltlab.wcf/update_2.1.0_alpha_1.sql b/com.woltlab.wcf/update_2.1.0_alpha_1.sql
index 7e53bdec6a..c73c93fe1a 100644
--- a/com.woltlab.wcf/update_2.1.0_alpha_1.sql
+++ b/com.woltlab.wcf/update_2.1.0_alpha_1.sql
@@ -251,7 +251,7 @@ ALTER TABLE wcf1_paid_subscription_transaction_log ADD FOREIGN KEY (userID) REFE
 ALTER TABLE wcf1_paid_subscription_transaction_log ADD FOREIGN KEY (subscriptionID) REFERENCES wcf1_paid_subscription (subscriptionID) ON DELETE SET NULL;
 ALTER TABLE wcf1_paid_subscription_transaction_log ADD FOREIGN KEY (paymentMethodObjectTypeID) REFERENCES wcf1_object_type (objectTypeID) ON DELETE CASCADE;
 
-ALTER TABLE wcf1_session_virtual ADD FOREIGN KEY (sessionID) REFERENCES wcf1_session (sessionID) ON DELETE CASCADE;
+ALTER TABLE wcf1_session_virtual ADD FOREIGN KEY (sessionID) REFERENCES wcf1_session (sessionID) ON DELETE CASCADE ON UPDATE CASCADE;
 
 ALTER TABLE wcf1_user_group_assignment ADD FOREIGN KEY (groupID) REFERENCES wcf1_user_group (groupID) ON DELETE CASCADE;
 
@@ -297,4 +297,4 @@ UPDATE wcf1_bbcode_media_provider SET html = '<iframe style="max-width:100%;" wi
  */
 
 /* change default value to '1' */
-UPDATE wcf1_like SET time = 1 WHERE time = 0;
\ No newline at end of file
+UPDATE wcf1_like SET time = 1 WHERE time = 0;
diff --git a/wcfsetup/install/files/lib/system/database/editor/MySQLDatabaseEditor.class.php b/wcfsetup/install/files/lib/system/database/editor/MySQLDatabaseEditor.class.php
index 240f56c979..e3e4ed17c9 100644
--- a/wcfsetup/install/files/lib/system/database/editor/MySQLDatabaseEditor.class.php
+++ b/wcfsetup/install/files/lib/system/database/editor/MySQLDatabaseEditor.class.php
@@ -164,6 +164,8 @@ class MySQLDatabaseEditor extends DatabaseEditor {
 		
 		// add operation and action
 		if (!empty($indexData['operation'])) $sql .= " ON ".$indexData['operation']." ".$indexData['action'];
+		if (!empty($indexData['ON DELETE'])) $sql .= " ON DELETE ".$indexData['ON DELETE'];
+		if (!empty($indexData['ON UPDATE'])) $sql .= " ON UPDATE ".$indexData['ON UPDATE'];
 		
 		$statement = $this->dbObj->prepareStatement($sql);
 		$statement->execute();
diff --git a/wcfsetup/install/files/lib/system/database/editor/PostgreSQLDatabaseEditor.class.php b/wcfsetup/install/files/lib/system/database/editor/PostgreSQLDatabaseEditor.class.php
index c072952571..e1ad85525b 100644
--- a/wcfsetup/install/files/lib/system/database/editor/PostgreSQLDatabaseEditor.class.php
+++ b/wcfsetup/install/files/lib/system/database/editor/PostgreSQLDatabaseEditor.class.php
@@ -301,6 +301,8 @@ class PostgreSQLDatabaseEditor extends DatabaseEditor {
 		
 		// add operation and action
 		if (!empty($indexData['operation'])) $sql .= " ON ".$indexData['operation']." ".$indexData['action'];
+		if (!empty($indexData['ON DELETE'])) $sql .= " ON DELETE ".$indexData['ON DELETE'];
+		if (!empty($indexData['ON UPDATE'])) $sql .= " ON UPDATE ".$indexData['ON UPDATE'];
 		
 		$statement = $this->dbObj->prepareStatement($sql);
 		$statement->execute();
diff --git a/wcfsetup/install/files/lib/system/database/util/SQLParser.class.php b/wcfsetup/install/files/lib/system/database/util/SQLParser.class.php
index 433f364eb2..c168898963 100644
--- a/wcfsetup/install/files/lib/system/database/util/SQLParser.class.php
+++ b/wcfsetup/install/files/lib/system/database/util/SQLParser.class.php
@@ -120,8 +120,14 @@ class SQLParser {
 					$this->executeAddIndexStatement($match[1], ($match[3] ?: self::getGenericIndexName($match[1], $match[4])), array('type' => strtoupper($match[2]), 'columns' => $match[4]));
 				}
 				// add foreign key
-				else if (preg_match('~^ALTER\s+TABLE\s+(\w+)\s+ADD\s+FOREIGN KEY\s+(?:(\w+)\s*)?\((\s*\w+\s*(?:,\s*\w+\s*)*)\)\s+REFERENCES\s+(\w+)\s+\((\s*\w+\s*(?:,\s*\w+\s*)*)\)(?:\s+ON\s+(UPDATE|DELETE)\s+(CASCADE|SET NULL|NO ACTION))?~is', $query, $match)) {
-					$this->executeAddForeignKeyStatement($match[1], ($match[2] ?: self::getGenericIndexName($match[1], $match[3], 'fk')), array('columns' => $match[3], 'referencedTable' => $match[4], 'referencedColumns' => $match[5], 'operation' => $match[6], 'action' => $match[7]));
+				else if (preg_match('~^ALTER\s+TABLE\s+(\w+)\s+ADD\s+FOREIGN KEY\s+(?:(\w+)\s*)?\((\s*\w+\s*(?:,\s*\w+\s*)*)\)\s+REFERENCES\s+(\w+)\s+\((\s*\w+\s*(?:,\s*\w+\s*)*)\)(?:\s+ON\s+DELETE\s+(CASCADE|SET NULL|NO ACTION))?(?:\s+ON\s+UPDATE\s+(CASCADE|SET NULL|NO ACTION))?~is', $query, $match)) {
+					$this->executeAddForeignKeyStatement($match[1], ($match[2] ?: self::getGenericIndexName($match[1], $match[3], 'fk')), array(
+						'columns' => $match[3],
+						'referencedTable' => $match[4],
+						'referencedColumns' => $match[5],
+						'ON DELETE' => $match[6],
+						'ON UPDATE' => $match[7]
+					));
 				}
 				// add/change column
 				else if (preg_match("~^ALTER\s+TABLE\s+(\w+)\s+(?:(ADD)\s+(?:COLUMN\s+)?|(CHANGE)\s+(?:COLUMN\s+)?(\w+)\s+)(\w+)\s+(\w+)(?:\s*\((\s*(?:\d+(?:\s*,\s*\d+)?|'[^']*'(?:\s*,\s*'[^']*')*))\s*\))?(?:\s+UNSIGNED)?(?:\s+(NOT NULL|NULL))?(?:\s+DEFAULT\s+(-?\d+.\d+|-?\d+|NULL|'[^'\\\\]*(?:\\\\.[^'\\\\]*)*'))?(?:\s+(AUTO_INCREMENT))?(?:\s+(UNIQUE|PRIMARY)(?: KEY)?)?~is", $query, $match)) {
diff --git a/wcfsetup/install/files/lib/system/session/SessionHandler.class.php b/wcfsetup/install/files/lib/system/session/SessionHandler.class.php
index 020c5ad7c0..c087d58bdd 100644
--- a/wcfsetup/install/files/lib/system/session/SessionHandler.class.php
+++ b/wcfsetup/install/files/lib/system/session/SessionHandler.class.php
@@ -185,6 +185,13 @@ class SessionHandler extends SingletonFactory {
 		// init session environment
 		$this->loadVariables();
 		$this->initSecurityToken();
+		
+		// session id change was delayed to the next request
+		// as the SID constants already were defined
+		if ($this->getVar('__changeSessionID')) {
+			$this->unregister('__changeSessionID');
+			$this->changeSessionID();
+		}
 		$this->defineConstants();
 		
 		// assign language and style id
@@ -195,6 +202,39 @@ class SessionHandler extends SingletonFactory {
 		$this->initEnvironment();
 	}
 	
+	/**
+	 * Changes the session id to a new random one.
+	 * 
+	 * Usually a change is requested after login to ensure
+	 * that the user is not running a fixated session by an
+	 * attacker.
+	 */
+	protected function changeSessionID() {
+		$oldSessionID = $this->session->sessionID;
+		$newSessionID = StringUtil::getRandomID();
+		
+		$sessionEditor = new $this->sessionEditorClassName($this->session);
+		$sessionEditor->update(array(
+			'sessionID' => $newSessionID
+		));
+		
+		// fetch new session data from database
+		$this->session = new $this->sessionClassName($newSessionID);
+		
+		if ($this->useCookies) {
+			// we know that the user accepts cookies, simply send new session id
+			HeaderUtil::setCookie('cookieHash', $newSessionID);
+		}
+		else if ($_SERVER['REQUEST_METHOD'] === 'GET') {
+			// user maybe does not accept cookies, replace session id in url
+			// otherwise reloading the page will generate a new session
+			
+			$this->update();
+			HeaderUtil::redirect(str_replace('s='.$oldSessionID, 's='.$newSessionID, UserUtil::getRequestURI()));
+			exit;
+		}
+	}
+	
 	/**
 	 * Enables cookie support.
 	 */
@@ -596,20 +636,10 @@ class SessionHandler extends SingletonFactory {
 				// update session
 				$sessionEditor = new $this->sessionEditorClassName($this->session);
 				
-				// regenerating the session id is essential to prevent session fixation attacks
-				// FIXME: but it cannot be used if cookies are not available, as the constants are already
-				// defined, erase security token in this case for basic security
-				if ($this->useCookies) {
-					$newSessionID = StringUtil::getRandomID();
-				}
-				else {
-					$this->unregister('__SECURITY_TOKEN');
-					$newSessionID = $this->session->sessionID;
-				}
+				$this->register('__changeSessionID', true);
 				
 				try {
 					$sessionEditor->update(array(
-						'sessionID' => $newSessionID,
 						'userID' => $user->userID
 					));
 				}
@@ -625,10 +655,9 @@ class SessionHandler extends SingletonFactory {
 									AND userID = ?";
 						$statement = WCF::getDB()->prepareStatement($sql);
 						$statement->execute(array($this->sessionID, $user->userID));
-							
+						
 						// update session
 						$sessionEditor->update(array(
-							'sessionID' => $newSessionID,
 							'userID' => $user->userID
 						));
 					}
@@ -637,10 +666,6 @@ class SessionHandler extends SingletonFactory {
 						throw $e;
 					}
 				}
-				
-				$this->session = new $this->sessionClassName($newSessionID);
-				
-				if ($this->useCookies) HeaderUtil::setCookie('cookieHash', $newSessionID);
 			}
 		}
 		
@@ -713,24 +738,11 @@ class SessionHandler extends SingletonFactory {
 					$sessionEditor = new $this->sessionEditorClassName($this->session);
 					
 					try {
-						// regenerating the session id is essential to prevent session fixation attacks
-						// FIXME: but it cannot be used if cookies are not available, as the constants are already
-						// defined, erase security token in this case for basic security
-						if ($this->useCookies) {
-							$newSessionID = StringUtil::getRandomID();
-						}
-						else {
-							$this->unregister('__SECURITY_TOKEN');
-							$newSessionID = $this->session->sessionID;
-						}
+						$this->register('__changeSessionID', true);
 						
 						$sessionEditor->update(array(
-							'sessionID' => $newSessionID,
 							'userID' => $user->userID
 						));
-						$this->session = new $this->sessionClassName($newSessionID);
-						
-						if ($this->useCookies) HeaderUtil::setCookie('cookieHash', $newSessionID);
 					}
 					catch (DatabaseException $e) {
 						// MySQL error 23000 = unique key
diff --git a/wcfsetup/setup/db/install.sql b/wcfsetup/setup/db/install.sql
index dd99d8dcaa..87e7948140 100644
--- a/wcfsetup/setup/db/install.sql
+++ b/wcfsetup/setup/db/install.sql
@@ -1580,7 +1580,7 @@ ALTER TABLE wcf1_search ADD FOREIGN KEY (userID) REFERENCES wcf1_user (userID) O
 ALTER TABLE wcf1_session ADD FOREIGN KEY (userID) REFERENCES wcf1_user (userID) ON DELETE CASCADE;
 ALTER TABLE wcf1_session ADD FOREIGN KEY (spiderID) REFERENCES wcf1_spider (spiderID) ON DELETE CASCADE;
 
-ALTER TABLE wcf1_session_virtual ADD FOREIGN KEY (sessionID) REFERENCES wcf1_session (sessionID) ON DELETE CASCADE;
+ALTER TABLE wcf1_session_virtual ADD FOREIGN KEY (sessionID) REFERENCES wcf1_session (sessionID) ON DELETE CASCADE ON UPDATE CASCADE;
 
 ALTER TABLE wcf1_sitemap ADD FOREIGN KEY (packageID) REFERENCES wcf1_package (packageID) ON DELETE CASCADE;
 
-- 
2.20.1