From 094d4d1db04e02f61cea96262c90f3875fb1b730 Mon Sep 17 00:00:00 2001
From: Marc Delisle <marc@infomarc.info>
Date: Fri, 29 Sep 2006 13:52:08 +0000
Subject: [PATCH] security fixes

---
 ChangeLog                        |  3 +++
 libraries/common.lib.php         | 34 +++++++++++++++++++++++---------
 libraries/session.inc.php        |  5 +++--
 libraries/url_generating.lib.php |  4 ++--
 4 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 91eb09509..c842fd3ff 100755
--- a/ChangeLog
+++ b/ChangeLog
@@ -10,6 +10,9 @@ $Source$
       thanks to Björn Wiberg - bwiberg.
     * libraries/grab_globals.lib.php: fix attack via _FILES,
       thanks to Stefan Esser
+    * libraries/common.lib.php, /session.inc.php, /url_generating.lib.php:
+      security fixes (announcement will come later),
+      thanks to Sebastian Mendel and Stefan Esser
 
 2006-09-27 Marc Delisle  <lem9@users.sourceforge.net>
     * libraries/.htaccess: remove potential vulnerability (allow from none),
diff --git a/libraries/common.lib.php b/libraries/common.lib.php
index 017874a21..470f68d1f 100644
--- a/libraries/common.lib.php
+++ b/libraries/common.lib.php
@@ -2435,6 +2435,17 @@ if (isset($_REQUEST['GLOBALS']) || isset($_FILES['GLOBALS'])
     die('GLOBALS overwrite attempt');
 }
 
+/**
+ * Check for numeric keys 
+ * (if register_globals is on, numeric key can be found in $GLOBALS)
+ */
+
+foreach ($GLOBALS as $key => $dummy) {
+    if (is_numeric($key)) {
+        die('numeric key detected');
+    }
+}
+
 /**
  * just to be sure there was no import (registering) before here
  * we empty the global space
@@ -2511,11 +2522,6 @@ if (get_magic_quotes_gpc()) {
     PMA_arrayWalkRecursive($_REQUEST, 'stripslashes', true);
 }
 
-/**
- * start session
- */
-require_once './libraries/session.inc.php';
-
 /**
  * include deprecated grab_globals only if required
  */
@@ -2523,6 +2529,11 @@ if (empty($__redirect) && !defined('PMA_NO_VARIABLES_IMPORT')) {
     require './libraries/grab_globals.lib.php';
 }
 
+/**
+ * include session handling after the globals, to prevent overwriting 
+ */
+require_once './libraries/session.inc.php';
+
 /**
  * init some variables LABEL_variables_init
  */
@@ -2642,13 +2653,13 @@ if (PMA_checkPageValidity($_REQUEST['back'], $goto_whitelist)) {
  * dangerous stuff from request.
  *
  * remember that some objects in the session with session_start and __wakeup()
- * could acces this variables before we reach this point
+ * could access this variables before we reach this point
  * f.e. PMA_Config: fontsize
  *
  * @todo variables should be handled by their respective owners (objects)
  * f.e. lang, server, convcharset, collation_connection in PMA_Config
  */
-if (! isset($_REQUEST['token']) || $_SESSION['PMA_token'] != $_REQUEST['token']) {
+if (empty($_REQUEST['token']) || $_SESSION[' PMA_token '] != $_REQUEST['token']) {
     /* List of parameters which are allowed from unsafe source */
     $allow_list = array(
         'db', 'table', 'lang', 'server', 'convcharset', 'collation_connection', 'target',
@@ -2660,7 +2671,12 @@ if (! isset($_REQUEST['token']) || $_SESSION['PMA_token'] != $_REQUEST['token'])
         'pma_servername', 'pma_username', 'pma_password',
     );
     /* Remove any non allowed stuff from requests */
-    foreach($_REQUEST as $key => $dummy) {
+    // do not check only $_REQUEST because it could have been overwritten
+    // and use type casting because the variables could have become 
+    // strings
+    $keys = array_keys(array_merge((array)$_REQUEST, (array)$_GET, (array)$_POST, (array)$_COOKIE));
+
+    foreach($keys as $key) {
         if (!in_array($key, $allow_list)) {
             unset($_REQUEST[$key], $_GET[$key], $_POST[$key], $GLOBALS[$key]);
         } else {
@@ -2668,7 +2684,7 @@ if (! isset($_REQUEST['token']) || $_SESSION['PMA_token'] != $_REQUEST['token'])
             $_REQUEST[$key] = htmlspecialchars($_REQUEST[$key], ENT_QUOTES);
         }
     }
-    unset($key, $dummy);
+    unset($key, $keys);
 }
 
 
diff --git a/libraries/session.inc.php b/libraries/session.inc.php
index 3b37c6a4a..14294c648 100644
--- a/libraries/session.inc.php
+++ b/libraries/session.inc.php
@@ -84,9 +84,10 @@ ini_set('session.save_handler', 'files');
 
 /**
  * Token which is used for authenticating access queries.
+ * (we use "space PMA_token space" to prevent overwriting)
  */
-if (!isset($_SESSION['PMA_token'])) {
-    $_SESSION['PMA_token'] = md5(uniqid(rand(), true));
+if (!isset($_SESSION[' PMA_token '])) {
+    $_SESSION[' PMA_token '] = md5(uniqid(rand(), true));
 }
 
 /**
diff --git a/libraries/url_generating.lib.php b/libraries/url_generating.lib.php
index 535b6e5a5..118be3cde 100644
--- a/libraries/url_generating.lib.php
+++ b/libraries/url_generating.lib.php
@@ -64,7 +64,7 @@ function PMA_generate_common_hidden_inputs($db = '', $table = '', $indent = 0, $
         $params['collation_connection'] = $GLOBALS['collation_connection'];
     }
 
-    $params['token'] = $_SESSION['PMA_token'];
+    $params['token'] = $_SESSION[' PMA_token '];
 
     if (! is_array($skip)) {
         if (isset($params[$skip])) {
@@ -182,7 +182,7 @@ function PMA_generate_common_url ($db = '', $table = '', $delim = '&amp;')
         $params['collation_connection'] = $GLOBALS['collation_connection'];
     }
 
-    $params['token'] = $_SESSION['PMA_token'];
+    $params['token'] = $_SESSION[' PMA_token '];
 
     $param_strings = array();
     foreach ($params as $key => $val) {