CiviCRM Community Forums (archive)

The initialize() function in CRM/Core/Session.php breaks Drupal sessions in a subtle but nasty way. Session.php currently ignores the fact that Drupal creates 'lazy sessions' for anonymous users.

A 'lazy session' only creates persistent data if there is any content in $_SESSION. Drupal checks this AFTER processing the page request, and if $_SESSION is not empty it adds a row to the Drupal sessions table and sets a session cookie in the page response.

Session.php on the other hand checks if $_SESSION is set BEFORE the page request is handled, and calls drupal_session_start() if it is unset. It fails to check if a 'lazy session' is in progress and fails to destroy the old session.

Impact

After the page request is finished there will be two rows in the Drupal sessions table that share the same ssid (secure session id). Drupal assumes there is only one, which leads to all sorts of subtle, odd behaviours.

Solution

Drupal needs the lazy session to continue, and CiviCRM needs $_SESSION to not be NULL and to be an array. Here is a patch ...

--- CRM/Core/Session.php 2014-02-28 17:58:18.128813697 +1100
+++ ../cbf/patches/CRM/Core/Session.php 2014-02-28 19:55:26.014398608 +1100
@@ -119,7 +119,10 @@
         }
         $config =& CRM_Core_Config::singleton();
         if ($config->userSystem->is_drupal && function_exists('drupal_session_start')) {
-          drupal_session_start();
+          if ($GLOBALS['lazy_session'] != true) {
+            drupal_session_start();
+          }
+          $_SESSION = array();
         }
         else {
           session_start();

To reproduce

We need an anonymous user, who has no session data on the server, accessing a vulnerable page via HTTPS.

• Run the securepages module and set it so civicrm pages are forced to go via HTTPS
• Using Chrome, ensure there are no Drupal session cookies in your browser
• Using a data admin tool, empty the Drupal sessions table
• Visit /civicrm/event/info
• There will be a SSESS* session cookie in the browser, but no SESS* cookie. There will be 2 rows in the Drupal sessions table whose ssid column matches the SSESS* cookie data. The expected behaviour is that there should be only one row in sessions where the sid column matches the SESS* cookie and the ssid column matches the SSESS* cookie.
• (If you don't see this result, try clearing cookies, the sessions table and refreshing.)
• Click on Register Now to go to /civicrm/event/register
• Enter your registration details and press Continue >>
• The expected outcome is that we go to the Confirm page, but the observed behaviour is that return to the Registration page but with the registration fields cleared

Thanks Ken for investigating and supplying a patch. Do you have any recommendations for testing it?

So far so good, since applying the patch, I have not observed any incidents (I have a script that analyses web server logs for the behaviour, i.e. redirection back to ?reset=1 after a POST, because the qfkey could not validate). c.f. https://github.com/mlutfy/nodisys-hosting-scripts/blob/master/nodisys-civicrm-contribute-analyis

I created a JIRA issue to track this bug: http://issues.civicrm.org/jira/browse/CRM-14356

@Ken: if you don't object, by request of Lobo, I will submit a pull-request with your patch so that we can integrate this in CiviCRM 4.4.5.

@colemanw,

I included steps to reproduce in my original post. If that's not working for you, I'm sure how else to trigger this.

@mathieu

"Should the $_SESSION be initialized only if we called drupal_session_start()? Otherwise, isn't there a risk of losing data set by Drupal or other?"

The thing is that ...

• This function has already tested that $_SESSION is not set, and I assume this is being done because the rest of CiviCRM requires that $_SESSION is set. Indeed, if not set, the line straight after this "$this->_session =& $_SESSION;" will bork.
• If you look at drupal_session_start(), it doesn't always set $_SESSION. So $_SESSION has to be set in this case, to be sure, to be sure.
• On the other hand, if a lazy session is in progress, we've already tested that $_SESSION is not set. We have to set it in this case.
• If the session is lazy, and $_SESSION remains an empty array until the end of the response to this request, Drupal will not persist the session

The setting of $_SESSION should remain as per my patch.

"Also, should we instead test using isset"

That's a good idea! $GLOBALS['lazy_session'] is either unset or TRUE, and your suggestion is better practice.

"pull-request"

Yes, please!

Joanne and Eileen think this has caused a regression -- http://forum.civicrm.org/index.php/topic,32409.0.html