From 921a3797403432ce33da9c4ed9130bb3c0b0445a Mon Sep 17 00:00:00 2001
From: Regis Houssin <regis@dolibarr.fr>
Date: Wed, 2 Nov 2011 20:24:51 +0100
Subject: [PATCH] Fix: security

---
 htdocs/admin/boxes.php             | 16 ++++++++-------
 htdocs/admin/ihm.php               | 21 +++++++++-----------
 htdocs/core/lib/usergroups.lib.php |  2 +-
 htdocs/main.inc.php                |  1 +
 htdocs/user/index.php              |  2 +-
 htdocs/user/info.php               |  8 ++++----
 htdocs/user/note.php               |  6 +++---
 htdocs/user/param_ihm.php          | 31 ++++++++++++++----------------
 8 files changed, 42 insertions(+), 45 deletions(-)

diff --git a/htdocs/admin/boxes.php b/htdocs/admin/boxes.php
index 45d003a6acc..2e9c2df0a92 100644
--- a/htdocs/admin/boxes.php
+++ b/htdocs/admin/boxes.php
@@ -28,8 +28,10 @@ include_once(DOL_DOCUMENT_ROOT."/core/lib/admin.lib.php");
 
 $langs->load("admin");
 
-if (!$user->admin)
-  accessforbidden();
+if (!$user->admin) accessforbidden();
+
+$rowid = GETPOST('rowid','int');
+$action = GETPOST('action');
 
 // Definition des positions possibles pour les boites
 $pos_array = array(0);                             // Positions possibles pour une boite (0,1,2,...)
@@ -39,12 +41,12 @@ $boxes = array();
 /*
  * Actions
  */
-if ((isset($_POST["action"]) && $_POST["action"] == 'addconst'))
+if ($action == 'addconst')
 {
     dolibarr_set_const($db, "MAIN_BOXES_MAXLINES",$_POST["MAIN_BOXES_MAXLINES"],'',0,'',$conf->entity);
 }
 
-if ($_POST["action"] == 'add')
+if ($action == 'add')
 {
 	$sql = "SELECT rowid";
 	$sql.= " FROM ".MAIN_DB_PREFIX."boxes";
@@ -95,12 +97,12 @@ if ($_POST["action"] == 'add')
 	}
 }
 
-if ($_GET["action"] == 'delete')
+if ($action == 'delete')
 {
 	$db->begin();
 
 	$sql = "DELETE FROM ".MAIN_DB_PREFIX."boxes";
-	$sql.= " WHERE rowid=".$_GET["rowid"];
+	$sql.= " WHERE rowid=".$rowid;
 	$resql = $db->query($sql);
 
 	// Remove all personalized setup when a box is activated or disabled
@@ -111,7 +113,7 @@ if ($_GET["action"] == 'delete')
 	$db->commit();
 }
 
-if ($_GET["action"] == 'switch')
+if ($action == 'switch')
 {
 	// On permute les valeur du champ box_order des 2 lignes de la table boxes
 	$db->begin();
diff --git a/htdocs/admin/ihm.php b/htdocs/admin/ihm.php
index e49910e2745..790341a9bd3 100644
--- a/htdocs/admin/ihm.php
+++ b/htdocs/admin/ihm.php
@@ -37,24 +37,21 @@ $langs->load("companies");
 $langs->load("products");
 $langs->load("members");
 
-if (!$user->admin)
-  accessforbidden();
+if (! $user->admin) accessforbidden();
+
+$action = GETPOST('action');
 
 
 if (! defined("MAIN_MOTD")) define("MAIN_MOTD","");
 
 // List of supported permanent search area
-$searchform=array(	"MAIN_SEARCHFORM_SOCIETE","MAIN_SEARCHFORM_CONTACT",
-					"MAIN_SEARCHFORM_PRODUITSERVICE","MAIN_SEARCHFORM_ADHERENT");
-$searchformconst=array($conf->global->MAIN_SEARCHFORM_SOCIETE,$conf->global->MAIN_SEARCHFORM_CONTACT,
-					$conf->global->MAIN_SEARCHFORM_PRODUITSERVICE,$conf->global->MAIN_SEARCHFORM_ADHERENT);
-$searchformtitle=array($langs->trans("Companies"),$langs->trans("Contacts"),
-					$langs->trans("ProductsAndServices"),$langs->trans("Members"));
-$searchformmodule=array('Module1Name','Module1Name',
-					'Module50Name','Module310Name');
+$searchform=array("MAIN_SEARCHFORM_SOCIETE","MAIN_SEARCHFORM_CONTACT", "MAIN_SEARCHFORM_PRODUITSERVICE","MAIN_SEARCHFORM_ADHERENT");
+$searchformconst=array($conf->global->MAIN_SEARCHFORM_SOCIETE,$conf->global->MAIN_SEARCHFORM_CONTACT,$conf->global->MAIN_SEARCHFORM_PRODUITSERVICE,$conf->global->MAIN_SEARCHFORM_ADHERENT);
+$searchformtitle=array($langs->trans("Companies"),$langs->trans("Contacts"),$langs->trans("ProductsAndServices"),$langs->trans("Members"));
+$searchformmodule=array('Module1Name','Module1Name','Module50Name','Module310Name');
 
 
-if (isset($_POST["action"]) && $_POST["action"] == 'update')
+if ($action == 'update')
 {
 	dolibarr_set_const($db, "MAIN_LANG_DEFAULT",       $_POST["main_lang_default"],'chaine',0,'',$conf->entity);
 	dolibarr_set_const($db, "MAIN_MULTILANGS",         $_POST["main_multilangs"],'chaine',0,'',$conf->entity);
@@ -102,7 +99,7 @@ print $langs->trans("DisplayDesc")."<br>\n";
 print "<br>\n";
 
 
-if (isset($_GET["action"]) && $_GET["action"] == 'edit')	// Edit
+if ($action == 'edit')	// Edit
 {
     print '<form method="post" action="'.$_SERVER["PHP_SELF"].'">';
     print '<input type="hidden" name="token" value="'.$_SESSION['newtoken'].'">';
diff --git a/htdocs/core/lib/usergroups.lib.php b/htdocs/core/lib/usergroups.lib.php
index cef75daef65..3eabf3291a7 100644
--- a/htdocs/core/lib/usergroups.lib.php
+++ b/htdocs/core/lib/usergroups.lib.php
@@ -233,7 +233,7 @@ function show_theme($fuser,$edit=0,$foruserprofile=false)
             $url=$urltheme."/".$subdir."/thumb.png";
             if (! file_exists($file)) $url=$urltheme."/common/nophoto.jpg";
             print '<table><tr><td>';
-			print '<a href="'.$_SERVER["PHP_SELF"].($edit?'?action=edit&theme=':'?theme=').$subdir.(! empty($_GET["optioncss"])?'&optioncss='.$_GET["optioncss"]:'').($fuser?'&id='.$fuser->id:'').'" style="font-weight: normal;" alt="'.$langs->trans("Preview").'">';
+			print '<a href="'.$_SERVER["PHP_SELF"].($edit?'?action=edit&theme=':'?theme=').$subdir.(GETPOST('optioncss')?'&optioncss='.GETPOST('optioncss'):'').($fuser?'&id='.$fuser->id:'').'" style="font-weight: normal;" alt="'.$langs->trans("Preview").'">';
 			if ($subdir == $conf->global->MAIN_THEME) $title=$langs->trans("ThemeCurrentlyActive");
 			else $title=$langs->trans("ShowPreview");
             print '<img src="'.$url.'" border="0" width="80" height="60" alt="'.$title.'" title="'.$title.'">';
diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php
index 99c32fbe253..85d41de3443 100644
--- a/htdocs/main.inc.php
+++ b/htdocs/main.inc.php
@@ -83,6 +83,7 @@ function test_sql_and_script_inject($val, $get)
 	$sql_inj += preg_match('/(\.\.%2f)+/i', $val);
 	// For XSS Injection done by adding javascript with script
 	$sql_inj += preg_match('/<script/i', $val);
+	$sql_inj += preg_match('/img[\s]src/i', $val);
 	// For XSS Injection done by adding javascript with onmousemove, etc... (closing a src or href tag with not cleaned param)
 	if ($get) $sql_inj += preg_match('/"/i', $val);	// We refused " in GET parameters value
 	return $sql_inj;
diff --git a/htdocs/user/index.php b/htdocs/user/index.php
index 13637d30ba7..d839736f5cb 100644
--- a/htdocs/user/index.php
+++ b/htdocs/user/index.php
@@ -36,7 +36,7 @@ $langs->load("companies");
 $socid=0;
 if ($user->societe_id > 0) $socid = $user->societe_id;
 
-$sall=isset($_GET["sall"])?$_GET["sall"]:$_POST["sall"];
+$sall=GETPOST('sall','alpha');
 
 $sortfield = GETPOST("sortfield",'alpha');
 $sortorder = GETPOST("sortorder",'alpha');
diff --git a/htdocs/user/info.php b/htdocs/user/info.php
index bb69ec84cca..db408b7a3b9 100644
--- a/htdocs/user/info.php
+++ b/htdocs/user/info.php
@@ -29,7 +29,7 @@ require_once(DOL_DOCUMENT_ROOT."/user/class/user.class.php");
 $langs->load("users");
 
 // Security check
-$id = isset($_GET["id"])?$_GET["id"]:'';
+$id = GETPOST('id');
 $fuser = new User($db);
 $fuser->fetch($id);
 
@@ -37,11 +37,11 @@ $fuser->fetch($id);
 $socid=0;
 if ($user->societe_id > 0) $socid = $user->societe_id;
 $feature2 = (($socid && $user->rights->user->self->creer)?'':'user');
-if ($user->id == $_GET["id"])	// A user can always read its own card
+if ($user->id == $id)	// A user can always read its own card
 {
 	$feature2='';
 }
-$result = restrictedArea($user, 'user', $_GET["id"], '', $feature2);
+$result = restrictedArea($user, 'user', $id, '', $feature2);
 
 // If user is not user read and no permission to read other users, we stop
 if (($fuser->id != $user->id) && (! $user->rights->user->user->lire))
@@ -55,7 +55,7 @@ if (($fuser->id != $user->id) && (! $user->rights->user->user->lire))
 
 llxHeader();
 
-$fuser->info($_GET["id"]);
+$fuser->info($id);
 
 $head = user_prepare_head($fuser);
 
diff --git a/htdocs/user/note.php b/htdocs/user/note.php
index 01fd68c6362..f29682e4db6 100644
--- a/htdocs/user/note.php
+++ b/htdocs/user/note.php
@@ -26,8 +26,8 @@ require("../main.inc.php");
 require_once(DOL_DOCUMENT_ROOT.'/core/lib/usergroups.lib.php');
 require_once(DOL_DOCUMENT_ROOT.'/user/class/user.class.php');
 
-$action=isset($_GET["action"])?$_GET["action"]:(isset($_POST["action"])?$_POST["action"]:"");
-$id=isset($_GET["id"])?$_GET["id"]:(isset($_POST["id"])?$_POST["id"]:"");
+$id = GETPOST('id');
+$action = GETPOST('action');
 
 $langs->load("companies");
 $langs->load("members");
@@ -57,7 +57,7 @@ $result = restrictedArea($user, 'user', $_GET["id"], '', $feature2);
 /*                     Actions                                                */
 /******************************************************************************/
 
-if ($_POST["action"] == 'update' && $user->rights->user->user->creer && ! $_POST["cancel"])
+if ($action == 'update' && $user->rights->user->user->creer && ! $_POST["cancel"])
 {
 	$db->begin();
 
diff --git a/htdocs/user/param_ihm.php b/htdocs/user/param_ihm.php
index 6c41f551fe1..0a8ecf02525 100644
--- a/htdocs/user/param_ihm.php
+++ b/htdocs/user/param_ihm.php
@@ -35,27 +35,28 @@ $langs->load("languages");
 // Defini si peux lire/modifier permisssions
 $canreaduser=($user->admin || $user->rights->user->user->lire);
 
-if ($_REQUEST["id"])
+$id = GETPOST('id');
+$action = GETPOST('action');
+
+if ($id)
 {
-    // $user est le user qui edite, $_REQUEST["id"] est l'id de l'utilisateur edite
-    $caneditfield=( (($user->id == $_REQUEST["id"]) && $user->rights->user->self->creer)
-    || (($user->id != $_REQUEST["id"]) && $user->rights->user->user->creer));
+    // $user est le user qui edite, $id est l'id de l'utilisateur edite
+    $caneditfield=( (($user->id == $id) && $user->rights->user->self->creer)
+    || (($user->id != $id) && $user->rights->user->user->creer));
 }
 
 // Security check
 $socid=0;
 if ($user->societe_id > 0) $socid = $user->societe_id;
 $feature2 = (($socid && $user->rights->user->self->creer)?'':'user');
-if ($user->id == $_REQUEST["id"])	// A user can always read its own card
+if ($user->id == $id)	// A user can always read its own card
 {
     $feature2='';
     $canreaduser=1;
 }
-$result = restrictedArea($user, 'user', $_REQUEST["id"], '', $feature2);
-if ($user->id <> $_REQUEST["id"] && ! $canreaduser) accessforbidden();
+$result = restrictedArea($user, 'user', $id, '', $feature2);
+if ($user->id <> $id && ! $canreaduser) accessforbidden();
 
-
-$id=! empty($_GET["id"])?$_GET["id"]:$_POST["id"];
 $dirtop = "../core/menus/standard";
 $dirleft = "../core/menus/standard";
 
@@ -76,13 +77,9 @@ $formadmin=new FormAdmin($db);
 /*
  * Actions
  */
-if ($_POST["action"] == 'update' && ($caneditfield  || $user->admin))
+if ($action == 'update' && ($caneditfield  || $user->admin))
 {
-    if ($_POST["cancel"])
-    {
-        $_GET["id"]=$_POST["id"];
-    }
-    else
+    if (! $_POST["cancel"])
     {
         $tabparam=array();
 
@@ -105,7 +102,7 @@ if ($_POST["action"] == 'update' && ($caneditfield  || $user->admin))
 
         $_SESSION["mainmenu"]="";   // Le gestionnaire de menu a pu changer
 
-        Header('Location: '.$_SERVER["PHP_SELF"].'?id='.$_POST["id"]);
+        Header('Location: '.$_SERVER["PHP_SELF"].'?id='.$id);
         exit;
     }
 }
@@ -151,7 +148,7 @@ if ($_GET["action"] == 'edit')
     print '<form method="post" action="'.$_SERVER["PHP_SELF"].'">';
     print '<input type="hidden" name="token" value="'.$_SESSION['newtoken'].'">';
     print '<input type="hidden" name="action" value="update">';
-    print '<input type="hidden" name="id" value="'.$_GET["id"].'">';
+    print '<input type="hidden" name="id" value="'.$id.'">';
 
     clearstatcache();
     $var=true;