Properly prevent session fixation in every case
authorTim Düsterhus <duesterhus@woltlab.com>
Tue, 9 Dec 2014 20:42:46 +0000 (21:42 +0100)
committerTim Düsterhus <duesterhus@woltlab.com>
Tue, 9 Dec 2014 20:47:09 +0000 (21:47 +0100)
com.woltlab.wcf/update_2.1.0_alpha_1.sql
wcfsetup/install/files/lib/system/database/editor/MySQLDatabaseEditor.class.php
wcfsetup/install/files/lib/system/database/editor/PostgreSQLDatabaseEditor.class.php
wcfsetup/install/files/lib/system/database/util/SQLParser.class.php
wcfsetup/install/files/lib/system/session/SessionHandler.class.php
wcfsetup/setup/db/install.sql

index 7e53bdec6a2fd02e641c4148512378819795a654..c73c93fe1a7c17dbf0f0e45778f5c62b7310d910 100644 (file)
@@ -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;
index 240f56c97932a89b08bd1e6e3dbbe922800a7fec..e3e4ed17c90f3f2f474de648521d4637efcebea8 100644 (file)
@@ -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();
index c072952571ec15d99b7c3ef24b1b199283821845..e1ad85525bebdd641637524e559c10be34689d88 100644 (file)
@@ -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();
index 433f364eb2c7ecdd1570f1b41ca1633faca0a7c9..c168898963b3b4d4580012bfe3555d05d99f16bd 100644 (file)
@@ -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)) {
index 020c5ad7c0e6d7873643f8a8727b6e8017f725c1..c087d58bdd3178d982b7e3631b31113e84b30170 100644 (file)
@@ -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
index dd99d8dcaaa134fc830bb149c4a9fa8d9bf3c934..87e7948140285496fdaded337c8ebb232b5b6fd9 100644 (file)
@@ -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;