From: Tim Düsterhus <duesterhus@woltlab.com>
Date: Wed, 14 Oct 2020 09:30:08 +0000 (+0200)
Subject: Sign the session cookie
X-Git-Tag: 5.4.0_Alpha_1~724^2~10^2~14
X-Git-Url: https://git.stricted.de/?a=commitdiff_plain;h=4ca13e5aa7470d2d76992be908ef8d9d04fea406;p=GitHub%2FWoltLab%2FWCF.git

Sign the session cookie
---

diff --git a/wcfsetup/install/files/lib/system/package/PackageInstallationDispatcher.class.php b/wcfsetup/install/files/lib/system/package/PackageInstallationDispatcher.class.php
index 80df31f82f..857e4c4699 100644
--- a/wcfsetup/install/files/lib/system/package/PackageInstallationDispatcher.class.php
+++ b/wcfsetup/install/files/lib/system/package/PackageInstallationDispatcher.class.php
@@ -36,6 +36,7 @@ use wcf\system\setup\Installer;
 use wcf\system\style\StyleHandler;
 use wcf\system\user\storage\UserStorageHandler;
 use wcf\system\WCF;
+use wcf\util\CryptoUtil;
 use wcf\util\FileUtil;
 use wcf\util\HeaderUtil;
 use wcf\util\JSON;
@@ -231,9 +232,14 @@ class PackageInstallationDispatcher {
 					]);
 					
 					$statement->execute([
-						\bin2hex(\random_bytes(20)),
+						$signatureSecret = \bin2hex(\random_bytes(20)),
 						'signature_secret'
 					]);
+					define('SIGNATURE_SECRET', $signatureSecret);
+					HeaderUtil::setCookie(
+						"acp_session",
+						CryptoUtil::createSignedString(WCF::getSession()->sessionID)
+					);
 					
 					if (WCF::getSession()->getVar('__wcfSetup_developerMode')) {
 						$statement->execute([
diff --git a/wcfsetup/install/files/lib/system/session/ACPSessionFactory.class.php b/wcfsetup/install/files/lib/system/session/ACPSessionFactory.class.php
index 3eb87e951e..d71820748f 100644
--- a/wcfsetup/install/files/lib/system/session/ACPSessionFactory.class.php
+++ b/wcfsetup/install/files/lib/system/session/ACPSessionFactory.class.php
@@ -55,16 +55,4 @@ class ACPSessionFactory {
 	protected function init() {
 		SessionHandler::getInstance()->initSession();
 	}
-	
-	/**
-	 * @deprecated 5.4 - Sessions are fully managed by SessionHandler.
-	 */
-	protected function readSessionID() {
-		// get sessionID from cookie
-		if (isset($_COOKIE[COOKIE_PREFIX.$this->cookieSuffix.'session'])) {
-			return $_COOKIE[COOKIE_PREFIX.$this->cookieSuffix.'session'];
-		}
-		
-		return '';
-	}
 }
diff --git a/wcfsetup/install/files/lib/system/session/SessionHandler.class.php b/wcfsetup/install/files/lib/system/session/SessionHandler.class.php
index 7664a91957..2868bcd781 100644
--- a/wcfsetup/install/files/lib/system/session/SessionHandler.class.php
+++ b/wcfsetup/install/files/lib/system/session/SessionHandler.class.php
@@ -16,6 +16,7 @@ use wcf\system\user\storage\UserStorageHandler;
 use wcf\system\SingletonFactory;
 use wcf\system\WCF;
 use wcf\system\WCFACP;
+use wcf\util\CryptoUtil;
 use wcf\util\HeaderUtil;
 use wcf\util\UserUtil;
 
@@ -179,6 +180,34 @@ final class SessionHandler extends SingletonFactory {
 	 */
 	public function setHasValidCookie($hasValidCookie) { }
 	
+	/**
+	 * Returns the session ID stored in the session cookie or `null`.
+	 */
+	private function getSessionIdFromCookie(): ?string {
+		$cookieName = COOKIE_PREFIX.($this->isACP ? 'acp' : 'user')."_session";
+		
+		if (isset($_COOKIE[$cookieName])) {
+			if (!PACKAGE_ID) {
+				return $_COOKIE[$cookieName];
+			}
+			
+			return CryptoUtil::getValueFromSignedString($_COOKIE[$cookieName]);
+		}
+		
+		return null;
+	}
+	
+	/**
+	 * Returns the signed session ID for use in a cookie.
+	 */
+	private function getSessionIdForCookie(string $sessionID): string {
+		if (!PACKAGE_ID) {
+			return $sessionID;
+		}
+		
+		return CryptoUtil::createSignedString($sessionID);
+	}
+	
 	/**
 	 * Returns true if client provided a valid session cookie.
 	 * 
@@ -186,10 +215,7 @@ final class SessionHandler extends SingletonFactory {
 	 * @since	3.0
 	 */
 	public function hasValidCookie(): bool {
-		$cookieName = COOKIE_PREFIX.($this->isACP ? 'acp' : 'user')."_session";
-		$sessionID = $_COOKIE[$cookieName] ?? null;
-		
-		return $sessionID === $this->sessionID;
+		return $this->getSessionIdFromCookie() === $this->sessionID;
 	}
 	
 	/**
@@ -211,8 +237,7 @@ final class SessionHandler extends SingletonFactory {
 	 * Loads the session matching the session cookie.
 	 */
 	public function loadFromCookie() {
-		$cookieName = COOKIE_PREFIX.($this->isACP ? 'acp' : 'user')."_session";
-		$sessionID = $_COOKIE[$cookieName] ?? null;
+		$sessionID = $this->getSessionIdFromCookie();
 		
 		$hasSession = false;
 		if ($sessionID) {
@@ -414,7 +439,11 @@ final class SessionHandler extends SingletonFactory {
 		
 		// Refresh cookie.
 		if ($this->user->userID && !$this->isACP) {
-			HeaderUtil::setCookie(($this->isACP ? 'acp' : 'user')."_session", $this->sessionID, TIME_NOW + 86400 * 14);
+			HeaderUtil::setCookie(
+				($this->isACP ? 'acp' : 'user')."_session",
+				$this->getSessionIdForCookie($this->sessionID),
+				TIME_NOW + 86400 * 14
+			);
 		}
 		
 		// Fetch legacy session.
@@ -470,7 +499,10 @@ final class SessionHandler extends SingletonFactory {
 			serialize([]),
 		]);
 		
-		HeaderUtil::setCookie(($this->isACP ? 'acp' : 'user')."_session", $this->sessionID);
+		HeaderUtil::setCookie(
+			($this->isACP ? 'acp' : 'user')."_session",
+			$this->getSessionIdForCookie($this->sessionID)
+		);
 		
 		$this->variables = [];
 		$this->user = new User(null);