Re: [SM-DEVEL] Re: [SM-USERS] session hijacking bug

[Rafael M. G. <r.m...@us...>]

On Mon, 2006-02-06 at 04:50, Iain Pople wrote:
> Hi Rafael,
> 

Hello

> Thanks for your post. I have some questions about your session handling 
> code.
> 
> Isn't it necessary to have some sort of transaction locking? When you 
> use file based sessions the OS automatically stops multiple processes 
> writing to the same session file. When I wrote a postgres session 
> handler i had the following:

With postgresql and MVCC (Multiversion Concurrency Control), you have
concurrency control when accesing the same table/row. I advise not to
use MySQL with MyISAM tables (as I have seen many places), the
performance in a busy system will be terrible because only one process
can access the table at the same time when updating data.

It is true that if you use a select to get some data that will be used
in a later update, you can get problems if two or more processes try to
do this and we do not use SELECT ... FOR UPDATE". Continue reading to
find out why we do not use it.

Some time ago, we tried a version of the session handler that did what
you suggest and we had big problems with transactions that were not
finnished and waiting for a commit. After a while all connections to the
database were in a 'waiting' state and the system stopped working
because we used max_connections to the DB.

I think the reason cause this happens is that a process can call to
sess_read() without having to call to sess_write() afterwards. The
problem I see with your example (and the one we used) is that we do not
have control over what happens between the call to sess_read() and the
call to sess_write() so we can not guarantee that an open transaction in
one connection will get commited later.


> 
>          function sess_read($id)
>          {
>            $this->db->query("BEGIN TRANSACTION");
>            ......................
>          }
> 

*** we do not have control over what happens here ****

>          function sess_write($id, $data)
>          {
>          
>                ....................
>             $this->db->query("COMMIT TRANSACTION");
>   
>          }
> 
> 
> This way the session could not be changed between reading and writing by 
> another process accessing the same session. Essentially this just 
> serializes accesses to the session.
>

As I said, we have been using the code I sent for a long time without a
problem and we generate several million transactions a day.

The only situation where it could not work properly will be if from the
same session/browser/client/user we generate to calls in parallel to the
session table, that need to run a select to get some data that will be
processed to update the table. 

I can not see how this can happen with squirrelmail. Anyone with any
input to this? 

> I also had to patch the PEAR DB.php code to force it to open a new psql 
> connection every time pg_connect is called. Otherwise the transaction 
> stuff above doesn't work. e.g.
> 
> pg_connect($dsn, PGSQL_CONNECT_FORCE_NEW)
> 

We use $db = DB::connect($dsn, false);. The false value will set the
second parameter to false and the system will use the defaults settings
(one of them (persistent boolean FALSE), so we do not use persistent
connections.

More information:
http://pear.php.net/manual/en/package.database.db.db.connect.php
http://pear.php.net/manual/en/package.database.db.db-common.setoption.php


> I also had to patch squirrelmail as there is a bug in PHP where the 
> session_destroy function clears the values set by 
> session_set_save_handler (http://bugs.php.net/bug.php?id=32330). I 
> assume you were including your code with a php_auto_prepend?
> 

We do not have problems with this at all(??), we are using php-4.3.11,
luck?. We define this in our php.ini: "session.save_handler = user" and
this in functions/global.php:

require_once("wwwsessions/db_sessions_v2.php");

after:

/** Bring in the config file. */
require_once(SM_PATH . 'config/config.php');

The session function I sent was not 100% correct. We are using it with
an extra line ($data = '';) after global $db;

****************************************************** 
//
// function: db_sess_read()
// purpose: Read the data of a session
//
function db_sess_read($sid){
  global $db;
  $data = '';

  $sql = "SELECT data FROM php_sessions WHERE sid = '$sid'";
  $result = $db->query($sql);
  
  if (DB::isError($result)) {
    $data = '';
  }
  else{
    $row = $result->fetchrow();
    $data = $row[0];
  }

  $result->free();
  return $data;
}
******************************************************

-- 
Rafael Martinez, <r.m...@us...>
Center for Information Technology Services
University of Oslo, Norway

PGP Public Key: http://folk.uio.no/rafael/