Finally fix up session restore functionality. Move session handling from login.php...

git-svn-id: https://svn.code.sf.net/p/squirrelmail/code/trunk/squirrelmail@12617 7612ce4b-ef26-0410-bec9-ea0150e637f0

diff --git a/include/init.php b/include/init.php
index cc2b8e43a2eae012b28e9ca86c394674a30c480b..226964729d8302a4dd903db500bdaec578a362e3 100644 (file)
--- a/include/init.php
+++ b/include/init.php
@@ -18,6 +18,13 @@
 error_reporting(E_ALL);
 
 
+/**
+ * Make sure we have a page name
+ *
+ */
+if ( !defined('PAGE_NAME') ) define('PAGE_NAME', NULL);
+
+
 /**
  * If register_globals are on, unregister globals.
  * Second test covers boolean set as string (php_value register_globals off).
@@ -199,18 +206,32 @@ if (!isset($session_name) || !$session_name) {
 }
 
 /**
- * if session.auto_start is On then close the session
+ * When on login page or if session.auto_start is On 
+ * we want to destroy/close the session (save off 
+ * possible session restoration values first)
  */
+if (!sqGetGlobalVar('session_expired_post', $sep, SQ_SESSION))
+    $sep = '';
+if (!sqGetGlobalVar('session_expired_location', $sel, SQ_SESSION))
+    $sel = '';
 $sSessionAutostartName = session_name();
 $sCookiePath = null;
-if ((isset($sSessionAutostartName) || $sSessionAutostartName == '') &&
-     $sSessionAutostartName !== $session_name) {
+if (PAGE_NAME == 'login' 
+ || (isset($sSessionAutostartName) && $sSessionAutostartName !== $session_name)) {
     $sCookiePath = ini_get('session.cookie_path');
     $sCookieDomain = ini_get('session.cookie_domain');
     // reset the cookie
     setcookie($sSessionAutostartName,'',time() - 604800,$sCookiePath,$sCookieDomain);
     @session_destroy();
     session_write_close();
+
+    /**
+     * in some rare instances, the session seems to stick
+     * around even after destroying it (!!), so if it does,
+     * we'll manually flatten the $_SESSION data
+     */
+    if (!empty($_SESSION))
+        $_SESSION = array();
 }
 
 /**
@@ -311,7 +332,6 @@ if (! sqgetGlobalVar('squirrelmail_language',$squirrelmail_language,SQ_COOKIE))
  * Do something special for some pages. This is based on the PAGE_NAME constand
  * set at the top of every page.
  */
-if ( !defined('PAGE_NAME') ) define('PAGE_NAME', NULL);
 switch (PAGE_NAME) {
     case 'style':
 
@@ -354,6 +374,13 @@ switch (PAGE_NAME) {
         require(SM_PATH . 'functions/page_header.php');
         require(SM_PATH . 'functions/html.php');
 
+        // put session restore data back into session if necessary
+        if (!empty($sel)) {
+            sqsession_register($sel, 'session_expired_location');
+            if (!empty($sep))
+                sqsession_register($sep, 'session_expired_post');
+        }
+
         // reset template file cache
         //
         $sTemplateID = Template::get_default_template_set();

diff --git a/src/compose.php b/src/compose.php
index bdbc3c404e8a98c4d398fab8a2ec71bb4c9eb2f5..52f9a489631e8e966aff3015545fb74b41f2e97d 100644 (file)
--- a/src/compose.php
+++ b/src/compose.php
@@ -329,11 +329,8 @@ if (sqsession_is_registered('session_expired_post')) {
      * extra check for username so we don't display previous post data from
      * another user during this session.
      */
-    if ($session_expired_post['username'] != $username) {
-        unset($session_expired_post);
-        sqsession_unregister('session_expired_post');
-        session_write_close();
-    } else {
+    if (!empty($session_expired_post['username']) 
+     && $session_expired_post['username'] == $username) {
         // these are the vars that we can set from the expired composed session
         $compo_var_list = array ('send_to', 'send_to_cc', 'body',
             'startMessage', 'passed_body', 'use_signature', 'signature',

diff --git a/src/login.php b/src/login.php
index 2eaea9e239ea6b4846a309aa2d5675c074dbf7d5..8888865b0016e050e8838cb3e5b97c4eca1a8d1f 100644 (file)
--- a/src/login.php
+++ b/src/login.php
@@ -30,34 +30,6 @@ require_once(SM_PATH . 'functions/forms.php');
  */
 set_up_language($squirrelmail_language, TRUE, TRUE);
 
-/**
- * In case the last session was not terminated properly, make sure
- * we get a new one, but make sure we preserve session_expired_*
- */
-$sep = '';
-$sel = '';
-sqGetGlobalVar('session_expired_post', $sep, SQ_SESSION);
-sqGetGlobalVar('session_expired_location', $sel, SQ_SESSION);
-
-/* blow away session */
-sqsession_destroy();
-
-/**
- * in some rare instances, the session seems to stick
- * around even after destroying it (!!), so if it does,
- * we'll manually flatten the $_SESSION data
- */
-if (!empty($_SESSION)) {
-    $_SESSION = array();
-}
-
-/* start session and put session_expired_* variables back in session */
-@sqsession_is_active();
-if (!empty($sep) && !empty($sel)) {
-    sqsession_register($sep, 'session_expired_post');
-    sqsession_register($sel, 'session_expired_location');
-}
-
 /**
  * This detects if the IMAP server has logins disabled, and if so,
  * squelches the display of the login form and puts up a message

diff --git a/src/redirect.php b/src/redirect.php
index cc2dde99ef55e55997ca827cb81ed9ebf2b0bbd3..6443ce5aa162012bad1da3d5a6e6fc754d5ce6d9 100644 (file)
--- a/src/redirect.php
+++ b/src/redirect.php
@@ -71,9 +71,10 @@ $imapConnection = sqimap_login($login_username, $key, $imapServerAddress, $imapP
 /* From now on we are logged it. If the login failed then sqimap_login handles it */
 
 /* regenerate the session id to avoid session hyijacking */
-sqsession_destroy();
-@sqsession_is_active();
-session_regenerate_id();
+//FIXME!  IMPORTANT!  SOMEONE PLEASE EXPLAIN THE SECURITY CONCERN HERE; THIS session_destroy() BORKS ANY SESSION INFORMATION ADDED ON THE LOGIN PAGE (SPECIFICALLY THE SESSION RESTORE DATA, BUT ALSO ANYTHING ADDED BY PLUGINS, ETC)... I HAVE DISABLED THIS (AND NOTE THAT THE LOGIN PAGE ALREADY EXECUTES A session_destroy() (see includes/init.php)), SO PLEASE, WHOEVER ADDED THIS, PLEASE ANALYSE THIS SITUATION AND COMMENT ON IF IT IS OK LIKE THIS!!  WHAT HIJACKING ISSUES ARE WE SUPPOSED TO BE PREVENTING HERE?
+//sqsession_destroy();
+//@sqsession_is_active();
+//session_regenerate_id();
 /**
 * The cookie part. session_start and session_regenerate_session normally set
 * their own cookie. SquirrelMail sets another cookie which overwites the
@@ -148,10 +149,12 @@ if ( sqgetGlobalVar('session_expired_location', $session_expired_location, SQ_SE
         if ($compose_new_win) {
             // do not prefix $location here because $session_expired_location is set to the PAGE_NAME
             // of the last page
-            $redirect_url = $session_expired_location.'.php';
+            $redirect_url = $location . $session_expired_location . '.php';
         } else {
-            $redirect_url = $location.'/webmail.php?right_frame='.urlencode($session_expired_location).'php';
+            $redirect_url = $location . '/webmail.php?right_frame=compose.php';
         }
+    } else {
+        $redirect_url = $location . '/webmail.php?right_frame=' . urlencode($session_expired_location) . '.php';
     }
     unset($session_expired_location);
 }