Improve security of sessions:

- use only cookies to store session id on client side
- use default php session system with database handler to store sessions on server side

git-svn-id: http://piwigo.org/svn/trunk@1004 68402e56-0260-453c-a942-63ccdbb3a9ee

include/category_calendar.inc.php

@@ -67,7 +67,6 @@ foreach ($calendar_years as $calendar_year => $nb_picture_year)
   {
     $url = PHPWG_ROOT_PATH.'category.php?cat=calendar';
     $url.= '&year='.$calendar_year;
-    $url = add_session_id($url);
     $years_nav_bar.= ' <a href="'.$url.'">'.$calendar_year.'</a>';
   }
 }
@@ -124,7 +123,7 @@ SELECT DISTINCT(MONTH('.$conf['calendar_datefield'].')) AS month
       $url = PHPWG_ROOT_PATH.'category.php?cat=calendar&amp;month=';
       $url.= $page['calendar_year'].'.'.sprintf('%02s', $calendar_month);
       $months_nav_bar.= ' ';
-      $months_nav_bar.= '<a href="'.add_session_id($url).'">';
+      $months_nav_bar.= '<a href="'.$url.'">';
       $months_nav_bar.= $lang['month'][(int)$calendar_month];
       $months_nav_bar.= '</a>';
     }
@@ -248,7 +247,7 @@ SELECT file,tn_ext,'.$conf['calendar_datefield'].',path
         'IMAGE_ALT'=>$row['file'],
         'IMAGE_TITLE'=>$thumbnail_title,
           
-        'U_IMG_LINK'=>add_session_id($url_link)
+        'U_IMG_LINK'=>$url_link
        )
      );
 
@@ -308,7 +307,7 @@ SELECT file,tn_ext,'.$conf['calendar_datefield'].',path
         'IMAGE_ALT'=>$row['file'],
         'IMAGE_TITLE'=>$thumbnail_title,
           
-        'U_IMG_LINK'=>add_session_id($url_link)
+        'U_IMG_LINK'=>$url_link
        )
      );
 
@@ -362,7 +361,7 @@ SELECT file,tn_ext,'.$conf['calendar_datefield'].',path
         'IMAGE_ALT'=>$row['file'],
         'IMAGE_TITLE'=>$thumbnail_title,
           
-        'U_IMG_LINK'=>add_session_id($url_link)
+        'U_IMG_LINK'=>$url_link
          )
        );
 
@@ -436,7 +435,7 @@ SELECT file,tn_ext,'.$conf['calendar_datefield'].',path
         'IMAGE_ALT'=>$row['file'],
         'IMAGE_TITLE'=>$thumbnail_title,
           
-        'U_IMG_LINK'=>add_session_id($url_link)
+        'U_IMG_LINK'=>$url_link
          )
        );
 

include/category_default.inc.php

@@ -89,7 +89,7 @@ while ($row = mysql_fetch_array($result))
       'IMAGE_TITLE'        => $thumbnail_title,
       'IMAGE_TS'           => get_icon($row['date_available']),
       
-      'U_IMG_LINK'         => add_session_id($url_link)
+      'U_IMG_LINK'         => $url_link
       )
     );
 

include/category_recent_cats.inc.php

@@ -78,7 +78,7 @@ while ( $row = mysql_fetch_array( $result ) )
       'IMAGE_ALT'               => $row['file'],
       'IMAGE_TITLE'             => $lang['hint_category'],
         
-      'U_IMG_LINK'              => add_session_id($url_link)
+      'U_IMG_LINK'              => $url_link
       )
     );
 

include/category_subcats.inc.php

@@ -152,7 +152,7 @@ SELECT id, path, tn_ext
         'IMAGE_TITLE' => $lang['hint_category'],
         'IMAGE_TS' => get_icon(@$item['date_last']),
         'U_IMG_LINK' =>
-          add_session_id(PHPWG_ROOT_PATH.'category.php?cat='.$item['category'])
+          PHPWG_ROOT_PATH.'category.php?cat='.$item['category']
         )
       );
     

include/config_default.inc.php

@@ -264,6 +264,21 @@ $conf['use_exif_mapping'] = array(
 // |                               sessions                                |
 // +-----------------------------------------------------------------------+
 
+// specifies to use cookie to store the session id on client side
+$conf['session_use_cookies'] = 1;
+
+// specifies to only use cookie to store the session id on client side
+$conf['session_use_only_cookies'] = 1;
+
+// do not use transparent session id support
+$conf['session_use_trans_sid'] = 0;
+
+// specifies the name of the session which is used as cookie name
+$conf['session_name'] = 'pwg_id';
+
+// comment the line below to use file handler for sessions.
+$conf['session_save_handler'] = 'db';
+
 // authorize_remembering : permits user to stay logged for a long time. It
 // creates a cookie on client side.
 $conf['authorize_remembering'] = true;
@@ -275,16 +290,6 @@ $conf['remember_me_length'] = 31536000;
 // session_length : time of validity for normal session, in seconds.
 $conf['session_length'] = 3600;
 
-// session_id_size : a session identifier is compound of alphanumeric
-// characters and is case sensitive. Each character is among 62
-// possibilities. The number of possible sessions is
-// 62^$conf['session_id_size'].
-//
-// 62^5  =             916,132,832
-// 62^10 = 839,299,365,868,340,224
-//
-$conf['session_id_size'] = 10;
-
 // +-----------------------------------------------------------------------+
 // |                                debug                                  |
 // +-----------------------------------------------------------------------+

include/functions_category.inc.php

@@ -47,7 +47,7 @@ function check_restrictions($category_id)
   if (in_array($category_id, explode(',', $user['forbidden_categories'])))
   {
     echo '<div style="text-align:center;">'.$lang['access_forbiden'].'<br />';
-    echo '<a href="'.add_session_id( './category.php' ).'">';
+    echo '<a href="./category.php">';
     echo $lang['thumbnails'].'</a></div>';
     exit();
   }

include/functions_html.inc.php

@@ -81,7 +81,7 @@ function create_navigation_bar($url, $nb_element, $start,
     if ($cur_page != 1)
     {
       $navbar.= '<a href="';
-      $navbar.= add_session_id($url.'&amp;start=0');
+      $navbar.= $url.'&amp;start=0';
       $navbar.= '" class="'.$link_class.'">'.$lang['first_page'];
       $navbar.= '</a>';
     }
@@ -95,7 +95,7 @@ function create_navigation_bar($url, $nb_element, $start,
     {
       $previous = $start - $nb_element_page;
       $navbar.= '<a href="';
-      $navbar.= add_session_id( $url.'&amp;start='.$previous );
+      $navbar.= $url.'&amp;start='.$previous;
       $navbar.= '" class="'.$link_class.'">'.$lang['previous_page'];
       $navbar.= '</a>';
     }
@@ -108,7 +108,7 @@ function create_navigation_bar($url, $nb_element, $start,
     if ($cur_page > $pages_around + 1)
     {
       $navbar.= '&nbsp;<a href="';
-      $navbar.= add_session_id($url.'&amp;start=0');
+      $navbar.= $url.'&amp;start=0';
       $navbar.= '" class="'.$link_class.'">1</a>';
       if ($cur_page > $pages_around + 2)
       {
@@ -129,7 +129,7 @@ function create_navigation_bar($url, $nb_element, $start,
       {
         $temp_start = ($i - 1) * $nb_element_page;
         $navbar.= '&nbsp;<a href="';
-        $navbar.= add_session_id($url.'&amp;start='.$temp_start);
+        $navbar.= $url.'&amp;start='.$temp_start;
         $navbar.= '" class="'.$link_class.'">'.$i.'</a>';
       }
       else
@@ -147,7 +147,7 @@ function create_navigation_bar($url, $nb_element, $start,
         $navbar.= ' ...';
       }
       $navbar.= ' <a href="';
-      $navbar.= add_session_id($url.'&amp;start='.$temp_start);
+      $navbar.= $url.'&amp;start='.$temp_start;
       $navbar.= '" class="'.$link_class.'">'.$maximum.'</a>';
     }
     
@@ -158,7 +158,7 @@ function create_navigation_bar($url, $nb_element, $start,
     {
       $next = $start + $nb_element_page;
       $navbar.= '<a href="';
-      $navbar.= add_session_id( $url.'&amp;start='.$next );
+      $navbar.= $url.'&amp;start='.$next;
       $navbar.= '" class="'.$link_class.'">'.$lang['next_page'].'</a>';
     }
     else
@@ -172,7 +172,7 @@ function create_navigation_bar($url, $nb_element, $start,
     {
       $temp_start = ($maximum - 1) * $nb_element_page;
       $navbar.= '<a href="';
-      $navbar.= add_session_id($url.'&amp;start='.$temp_start);
+      $navbar.= $url.'&amp;start='.$temp_start;
       $navbar.= '" class="'.$link_class.'">'.$lang['last_page'];
       $navbar.= '</a>';
     }
@@ -241,7 +241,7 @@ function get_cat_display_name($cat_informations,
     else
     {
       $output.= '<a class=""';
-      $output.= ' href="'.add_session_id(PHPWG_ROOT_PATH.$url.$id).'">';
+      $output.= ' href="'.PHPWG_ROOT_PATH.$url.$id.'">';
       $output.= $name.'</a>';
     }
   }
@@ -310,7 +310,7 @@ SELECT id,name
     {
       $output.= '
 <a class=""
-   href="'.add_session_id(PHPWG_ROOT_PATH.$url.$category_id).'">'.$name.'</a>';
+   href="'.PHPWG_ROOT_PATH.$url.$category_id.'">'.$name.'</a>';
     }
   }
   if ($replace_space)
@@ -369,7 +369,7 @@ function get_html_menu_category($categories)
     }
     $menu.= '>';
   
-    $url = add_session_id(PHPWG_ROOT_PATH.'category.php?cat='.$category['id']);
+    $url = PHPWG_ROOT_PATH.'category.php?cat='.$category['id'];
     $menu.= "\n".'<a href="'.$url.'">'.$category['name'].'</a>';
 
     if ($category['nb_images'] > 0)

include/functions_session.inc.php

@@ -25,115 +25,77 @@
 // | USA.                                                                  |
 // +-----------------------------------------------------------------------+
 
-// The function generate_key creates a string with pseudo random characters.
-// the size of the string depends on the $conf['session_id_size'].
-// Characters used are a-z A-Z and numerical values. Examples :
-//                    "Er4Tgh6", "Rrp08P", "54gj"
-// input  : none (using global variable)
-// output : $key
-function generate_key($size)
-{
-  global $conf;
-
-  $md5 = md5(substr(microtime(), 2, 6));
-  $init = '';
-  for ( $i = 0; $i < strlen( $md5 ); $i++ )
-  {
-    if ( is_numeric( $md5[$i] ) ) $init.= $md5[$i];
-  }
-  $init = substr( $init, 0, 8 );
-  mt_srand( $init );
-  $key = '';
-  for ( $i = 0; $i < $size; $i++ )
-  {
-    $c = mt_rand( 0, 2 );
-    if ( $c == 0 )      $key .= chr( mt_rand( 65, 90 ) );
-    else if ( $c == 1 ) $key .= chr( mt_rand( 97, 122 ) );
-    else                $key .= mt_rand( 0, 9 );
-  }
-  return $key;
+if (isset($conf['session_save_handler']) and ($conf['session_save_handler'] == 'db')) {
+  session_set_save_handler('pwg_session_open', 
+			   'pwg_session_close',
+			   'pwg_session_read',
+			   'pwg_session_write',
+			   'pwg_session_destroy',
+			   'pwg_session_gc'
+			   );
 }
 
-/**
- * create a new session and returns the session identifier
- *
- * - find a non-already-used session key
- * - create a session in database
- * - return session identifier
- *
- * @param int userid
- * @param int session_lentgh : in seconds
- * @return string
- */
-function session_create($userid, $session_length)
-{
-  global $conf;
+ini_set('session.use_cookies', $conf['session_use_cookies']);
+ini_set('session.use_only_cookies', $conf['session_use_only_cookies']);
+ini_set('session.use_trans_sid', $conf['session_use_trans_sid']);
+ini_set('session.name', $conf['session_name']);
 
-  // 1. searching an unused session key
-  $id_found = false;
-  while (!$id_found)
-  {
-    $generated_id = generate_key($conf['session_id_size']);
-    $query = '
-SELECT id
-  FROM '.SESSIONS_TABLE.'
-  WHERE id = \''.$generated_id.'\'
-;';
-    $result = pwg_query($query);
-    if (mysql_num_rows($result) == 0)
-    {
-      $id_found = true;
-    }
+function pwg_session_open($path, $name) 
+{
+  return true;
+}
+
+function pwg_session_close() 
+{
+  pwg_session_gc();
+  return true;
+}
+
+function pwg_session_read($session_id) 
+{
+  $query = "SELECT data FROM " . SESSIONS_TABLE;
+  $query .= " WHERE id = '$session_id'";
+  $result = pwg_query($query);
+  if ($result) {
+    $row = mysql_fetch_assoc($result);
+    return $row['data'];
+  } else {
+    return '';
   }
-  // 3. inserting session in database
-  $query = '
-INSERT INTO '.SESSIONS_TABLE.'
-  (id,user_id,expiration)
-  VALUES
-  (\''.$generated_id.'\','.$userid.',
-   ADDDATE(NOW(), INTERVAL '.$session_length.' SECOND))
-;';
+}
+
+function pwg_session_write($session_id, $data) 
+{
+  $query = "SELECT id FROM " . SESSIONS_TABLE;
+  $query .= " WHERE id = '$session_id'";
+  $result = pwg_query($query);
+  if (mysql_num_rows($result)) {
+    $query = "UPDATE " . SESSIONS_TABLE . " SET expiration = now()";
+    $query .= " WHERE id = '$session_id'";    
+    pwg_query($query);
+  } else {
+    $query = "INSERT INTO " . SESSIONS_TABLE . " (id,data,expiration)";
+    $query .= " VALUES('$session_id','$data',now())";
+    pwg_query($query);    
+  }
+  return true;
+}
+
+function pwg_session_destroy($session_id) 
+{
+  $query = "DELETE FROM " . SESSIONS_TABLE;
+  $query .= " WHERE id = '$session_id'";
   pwg_query($query);
-
-  $expiration = $session_length + time();
-  setcookie('id', $generated_id, $expiration, cookie_path());
-                
-  return $generated_id;
+  return true;
 }
 
-// add_session_id adds the id of the session to the string given in
-// parameter as $url. If the session id is the first parameter to the url,
-// it is preceded by a '?', else it is preceded by a '&amp;'. If the
-// parameter $redirect is set to true, '&' is used instead of '&'.
-function add_session_id( $url, $redirect = false )
+function pwg_session_gc() 
 {
-  global $page, $user, $conf;
+  global $conf;
 
-  if ($user['is_the_guest']
-      or $user['has_cookie']
-      or $conf['apache_authentication'])
-  {
-    return $url;
-  }
-
-  if (preg_match('/\.php\?/', $url))
-  {
-    $separator = $redirect ? '&' : '&amp;';
-  }
-  else
-  {
-    $separator = '?';
-  }
-
-  return $url.$separator.'id='.$page['session_id'];
-}
-
-// cookie_path returns the path to use for the PhpWebGallery cookie.
-// If PhpWebGallery is installed on :
-// http://domain.org/meeting/gallery/category.php
-// cookie_path will return : "/meeting/gallery"
-function cookie_path()
-{
-  return substr($_SERVER['PHP_SELF'],0,strrpos( $_SERVER['PHP_SELF'],'/'));
+  $query = "DELETE FROM " . SESSIONS_TABLE;
+  $query .= " WHERE UNIX_TIMESTAMP(NOW()) - UNIX_TIMESTAMP(expiration) > " . $conf['session_length'];
+  pwg_query($query);
+  return true;
 }
 ?>

include/page_header.php

@@ -61,7 +61,7 @@ if ( isset( $refresh ) and intval($refresh) >= 0 and isset( $url_link ) )
   $template->assign_vars(
     array(
       'REFRESH_TIME' => $refresh,
-      'U_REFRESH' => add_session_id( $url_link )
+      'U_REFRESH' => $url_link
       ));
   $template->assign_block_vars('refresh', array());
 }

include/user.inc.php

@@ -26,55 +26,24 @@
 // +-----------------------------------------------------------------------+
 
 // retrieving connected user informations
-if (isset($_COOKIE['id']))
+if (isset($_COOKIE[session_name()])) 
 {
-  $session_id = $_COOKIE['id'];
-  $user['has_cookie'] = true;
-}
-else if (isset($_GET['id']))
+ session_start();
+ if (isset($_SESSION['id'])) 
+ {
+   $user['id'] = $_SESSION['id'];    
+ }
+ else 
+ {
+   // session timeout
+   $user['id'] = $conf['guest_id'];
+   $user['is_the_guest'] = true;
+ }
+} 
+else 
 {
-  $session_id = $_GET['id'];
-  $user['has_cookie'] = false;
-}
-else
-{
-  $user['has_cookie'] = false;
-}
-
-if (isset($session_id)
-    and ereg("^[0-9a-zA-Z]{".$conf['session_id_size']."}$", $session_id))
-{
-  $page['session_id'] = $session_id;
-  $query = '
-SELECT user_id,expiration,NOW() AS now
-  FROM '.SESSIONS_TABLE.'
-  WHERE id = \''.$page['session_id'].'\'
-;';
-  $result = pwg_query($query);
-  if (mysql_num_rows($result) > 0)
-  {
-    $row = mysql_fetch_array($result);
-    if (strnatcmp($row['expiration'], $row['now']) < 0)
-    {
-      // deletion of the session from the database, because it is
-      // out-of-date
-      $delete_query = '
-DELETE FROM '.SESSIONS_TABLE.'
-  WHERE id = \''.$page['session_id'].'\'
-;';
-      pwg_query($delete_query);
-    }
-    else
-    {
-      $user['id'] = $row['user_id'];
-      $user['is_the_guest'] = false;
-    }
-  }
-}
-if (!isset($user['id']))
-{
-  $user['id'] = $conf['guest_id'];
-  $user['is_the_guest'] = true;
+ $user['id'] = $conf['guest_id'];
+ $user['is_the_guest'] = true;
 }
 
 // using Apache authentication override the above user search