Move the security token storage into a signed cookie

diff --git a/wcfsetup/install/files/lib/system/session/SessionHandler.class.php b/wcfsetup/install/files/lib/system/session/SessionHandler.class.php
index 8f899fe1a474df7fe0dff25fc37e87c070b6e12f..7f93fcc6cda21d18e674b35d563912ee021805bc 100644 (file)
--- a/wcfsetup/install/files/lib/system/session/SessionHandler.class.php
+++ b/wcfsetup/install/files/lib/system/session/SessionHandler.class.php
@@ -4,6 +4,7 @@ use wcf\data\session\Session;
 use wcf\data\session\SessionEditor;
 use wcf\data\user\User;
 use wcf\data\user\UserEditor;
+use wcf\system\application\ApplicationHandler;
 use wcf\system\cache\builder\SpiderCacheBuilder;
 use wcf\system\cache\builder\UserGroupOptionCacheBuilder;
 use wcf\system\cache\builder\UserGroupPermissionCacheBuilder;
@@ -12,6 +13,7 @@ use wcf\system\database\util\PreparedStatementConditionBuilder;
 use wcf\system\event\EventHandler;
 use wcf\system\exception\PermissionDeniedException;
 use wcf\system\page\PageLocationManager;
+use wcf\system\request\RouteHandler;
 use wcf\system\user\storage\UserStorageHandler;
 use wcf\system\SingletonFactory;
 use wcf\system\WCF;
@@ -125,6 +127,11 @@ final class SessionHandler extends SingletonFactory {
         */
        protected $usersOnlyPermissions = [];
        
+       /**
+        * @var string
+        */
+       private $xsrfToken;
+       
        private const ACP_SESSION_LIFETIME = 7200;
        private const GUEST_SESSION_LIFETIME = 7200;
        private const USER_SESSION_LIFETIME = 86400 * 14;
@@ -301,9 +308,49 @@ final class SessionHandler extends SingletonFactory {
         * Initializes security token.
         */
        protected function initSecurityToken() {
-               if ($this->getVar('__SECURITY_TOKEN') === null) {
-                       $this->register('__SECURITY_TOKEN', \bin2hex(\random_bytes(20)));
+               $xsrfToken = '';
+               if (!empty($_COOKIE['XSRF-TOKEN'])) {
+                       // We intentionally do not extract the signed value and instead just verify the correctness.
+                       //
+                       // The reason is that common JavaScript frameworks can use the contents of the `XSRF-TOKEN` cookie as-is,
+                       // without performing any processing on it, improving interoperability. Leveraging that JavaScript framework
+                       // feature requires the author of the controller to check the value within the `X-XSRF-TOKEN` request header
+                       // instead of the WoltLab Suite specific `t` parameter, though.
+                       //
+                       // The only reason we sign the cookie is that an XSS vulnerability or a rogue application on a subdomain
+                       // is not able to create a valid `XSRF-TOKEN`, e.g. by setting the `XSRF-TOKEN` cookie to the static
+                       // value `1234`, possibly allowing later exploitation.
+                       if (CryptoUtil::validateSignedString($_COOKIE['XSRF-TOKEN'])) {
+                               $xsrfToken = $_COOKIE['XSRF-TOKEN'];
+                       }
+               }
+               
+               if (!$xsrfToken) {
+                       $xsrfToken = CryptoUtil::createSignedString(\bin2hex(\random_bytes(20)));
+                       
+                       // We construct the cookie manually instead of using HeaderUtil::setCookie(), because:
+                       // 1) We don't want the prefix. The `XSRF-TOKEN` cookie name is a standard name across applications
+                       //    and it is supported by default in common JavaScript frameworks.
+                       // 2) We want to set the SameSite=strict parameter.
+                       // 3) We don't want the HttpOnly parameter.
+                       $sameSite = $cookieDomain = '';
+                       
+                       if (ApplicationHandler::getInstance()->isMultiDomainSetup()) {
+                               // We need to specify the cookieDomain in a multi domain set-up, because
+                               // otherwise no cookies are sent to subdomains.
+                               $cookieDomain = HeaderUtil::getCookieDomain();
+                               $cookieDomain = ($cookieDomain !== null ? '; domain='.$cookieDomain : '');
+                       }
+                       else {
+                               // SameSite=strict is not supported in a multi domain set-up, because
+                               // it breaks cross-application requests.
+                               $sameSite = '; SameSite=strict';
+                       }
+                       
+                       header('set-cookie: XSRF-TOKEN='.rawurlencode($xsrfToken).'; path=/'.$cookieDomain.(RouteHandler::secureConnection() ? '; secure' : '').$sameSite, false);
                }
+               
+               $this->xsrfToken = $xsrfToken;
        }
        
        /**
@@ -312,7 +359,7 @@ final class SessionHandler extends SingletonFactory {
         * @return      string
         */
        public function getSecurityToken() {
-               return $this->getVar('__SECURITY_TOKEN');
+               return $this->xsrfToken;
        }
        
        /**
@@ -323,6 +370,10 @@ final class SessionHandler extends SingletonFactory {
         * @return      boolean
         */
        public function checkSecurityToken($token) {
+               // The output of CryptoUtil::createSignedString() is not url-safe. For compatibility
+               // reasons the SECURITY_TOKEN in URLs might not be encoded, turning the '+' into a space.
+               // Convert it back before comparing.
+               $token = \str_replace(' ', '+', $token);
                return \hash_equals($this->getSecurityToken(), $token);
        }
        

diff --git a/wcfsetup/install/files/lib/util/HeaderUtil.class.php b/wcfsetup/install/files/lib/util/HeaderUtil.class.php
index 05e1de49fa241ff8731bfba8fdd8ddda576227d1..827970497a5e033b7cb2d77810a19b07c8b30ec0 100644 (file)
--- a/wcfsetup/install/files/lib/util/HeaderUtil.class.php
+++ b/wcfsetup/install/files/lib/util/HeaderUtil.class.php
@@ -2,6 +2,7 @@
 namespace wcf\util;
 use wcf\system\application\ApplicationHandler;
 use wcf\system\event\EventHandler;
+use wcf\system\exception\SystemException;
 use wcf\system\request\RequestHandler;
 use wcf\system\request\RouteHandler;
 use wcf\system\session\SessionHandler;
@@ -42,14 +43,28 @@ final class HeaderUtil {
         * @param       integer         $expire
         */
        public static function setCookie($name, $value = '', $expire = 0) {
+               $cookieDomain = self::getCookieDomain();
+               
+               @header('Set-Cookie: '.rawurlencode(COOKIE_PREFIX.$name).'='.rawurlencode((string) $value).($expire ? '; expires='.gmdate('D, d-M-Y H:i:s', $expire).' GMT; max-age='.($expire - TIME_NOW) : '').'; path=/'.($cookieDomain !== null ? '; domain='.$cookieDomain : '').(RouteHandler::secureConnection() ? '; secure' : '').'; HttpOnly', false);
+       }
+       
+       /**
+        * Returns the cookie domain for the active application or 'null' if no domain should be specified.
+        */
+       public static function getCookieDomain(): ?string {
                $application = ApplicationHandler::getInstance()->getActiveApplication();
                $addDomain = (mb_strpos($application->cookieDomain, '.') === false || StringUtil::endsWith($application->cookieDomain, '.lan') || StringUtil::endsWith($application->cookieDomain, '.local')) ? false : true;
+               
+               if (!$addDomain) {
+                       return null;
+               }
+               
                $cookieDomain = $application->cookieDomain;
                if ($addDomain && strpos($cookieDomain, ':') !== false) {
                        $cookieDomain = explode(':', $cookieDomain, 2)[0];
                }
                
-               @header('Set-Cookie: '.rawurlencode(COOKIE_PREFIX.$name).'='.rawurlencode((string) $value).($expire ? '; expires='.gmdate('D, d-M-Y H:i:s', $expire).' GMT; max-age='.($expire - TIME_NOW) : '').'; path=/'.($addDomain ? '; domain='.$cookieDomain : '').(RouteHandler::secureConnection() ? '; secure' : '').'; HttpOnly', false);
+               return $cookieDomain;
        }
        
        /**