From 7e94d8ae522bd7bc91935da70218aa4c9478ad98 Mon Sep 17 00:00:00 2001 From: Regis Houssin Date: Sat, 20 Oct 2012 09:05:24 +0200 Subject: [PATCH] Fix: minor vulnerabilities --- htdocs/admin/modules.php | 13 ++++++----- htdocs/externalsite/frames.php | 10 ++++----- htdocs/index.php | 2 +- htdocs/user/class/user.class.php | 32 +++++++++++++-------------- htdocs/user/class/usergroup.class.php | 28 +++++++++++------------ htdocs/user/group/perms.php | 15 +++++++------ htdocs/user/perms.php | 7 +++--- 7 files changed, 55 insertions(+), 52 deletions(-) diff --git a/htdocs/admin/modules.php b/htdocs/admin/modules.php index 184e679e1fc..f23713557d9 100644 --- a/htdocs/admin/modules.php +++ b/htdocs/admin/modules.php @@ -31,9 +31,10 @@ require_once(DOL_DOCUMENT_ROOT."/core/lib/admin.lib.php"); $langs->load("errors"); $langs->load("admin"); -$mode=isset($_GET["mode"])?GETPOST("mode"):(isset($_SESSION['mode'])?$_SESSION['mode']:0); +$mode=GETPOST('mode', 'alpha')?GETPOST('mode', 'alpha'):(isset($_SESSION['mode'])?$_SESSION['mode']:0); $mesg=GETPOST("mesg"); -$action=GETPOST('action'); +$action=GETPOST('action', 'alpha'); +$value=GETPOST('value', 'alpha'); if (!$user->admin) accessforbidden(); @@ -44,7 +45,7 @@ if (!$user->admin) accessforbidden(); if ($action == 'set' && $user->admin) { - $result=activateModule($_GET["value"]); + $result=activateModule($value); $mesg=''; if ($result) $mesg=$result; Header("Location: modules.php?mode=".$mode."&mesg=".urlencode($mesg)); @@ -53,7 +54,7 @@ if ($action == 'set' && $user->admin) if ($action == 'reset' && $user->admin) { - $result=unActivateModule($_GET["value"]); + $result=unActivateModule($value); $mesg=''; if ($result) $mesg=$result; Header("Location: modules.php?mode=".$mode."&mesg=".urlencode($mesg)); @@ -128,9 +129,9 @@ foreach ($modulesdir as $dir) { $mesg="Error: Module ".$modName." was found twice: Into ".$modNameLoaded[$modName]." and ".$dir.". You probably have an old file on your disk.
"; dol_syslog($mesg, LOG_ERR); - continue; + continue; } - + try { $res=include_once($dir.$file); diff --git a/htdocs/externalsite/frames.php b/htdocs/externalsite/frames.php index 64c88630fbe..dcccde6a2cc 100644 --- a/htdocs/externalsite/frames.php +++ b/htdocs/externalsite/frames.php @@ -34,11 +34,11 @@ if (empty($conf->global->EXTERNALSITE_URL)) llxFooter(); } -$mainmenu=GETPOST('mainmenu'); -$leftmenu=GETPOST('leftmenu'); -$idmenu=GETPOST('idmenu'); -$theme=GETPOST('theme'); -$codelang=GETPOST('lang'); +$mainmenu=GETPOST('mainmenu', 'alpha'); +$leftmenu=GETPOST('leftmenu', 'alpha'); +$idmenu=GETPOST('idmenu', 'int'); +$theme=GETPOST('theme', 'alpha'); +$codelang=GETPOST('lang', 'alpha'); print " diff --git a/htdocs/index.php b/htdocs/index.php index 0cad4bac8f1..18540d543f8 100644 --- a/htdocs/index.php +++ b/htdocs/index.php @@ -30,7 +30,7 @@ require_once(DOL_DOCUMENT_ROOT."/core/class/html.formother.class.php"); // If not defined, we select menu "home" -if (! isset($_GET["mainmenu"])) $_GET["mainmenu"]="home"; +$_GET['mainmenu']=GETPOST('mainmenu', 'alpha')?GETPOST('mainmenu', 'alpha'):'home'; $action=GETPOST('action'); diff --git a/htdocs/user/class/user.class.php b/htdocs/user/class/user.class.php index dc185404cb8..1eb8d0af40a 100644 --- a/htdocs/user/class/user.class.php +++ b/htdocs/user/class/user.class.php @@ -282,7 +282,7 @@ class User extends CommonObject * @param string $allperms Ajouter tous les droits du module allmodule, perms allperms * @return int > 0 if OK, < 0 if KO */ - function addrights($rid,$allmodule='',$allperms='') + function addrights($rid, $allmodule='', $allperms='') { global $conf; @@ -292,13 +292,13 @@ class User extends CommonObject $this->db->begin(); - if ($rid) + if (! empty($rid)) { // Si on a demande ajout d'un droit en particulier, on recupere // les caracteristiques (module, perms et subperms) de ce droit. $sql = "SELECT module, perms, subperms"; $sql.= " FROM ".MAIN_DB_PREFIX."rights_def"; - $sql.= " WHERE id = '".$rid."'"; + $sql.= " WHERE id = '".$this->escape($rid)."'"; $sql.= " AND entity = ".$conf->entity; $result=$this->db->query($sql); @@ -314,21 +314,21 @@ class User extends CommonObject } // Where pour la liste des droits a ajouter - $whereforadd="id=".$rid; + $whereforadd="id=".$this->escape($rid); // Ajout des droits induits - if ($subperms) $whereforadd.=" OR (module='$module' AND perms='$perms' AND (subperms='lire' OR subperms='read'))"; - else if ($perms) $whereforadd.=" OR (module='$module' AND (perms='lire' OR perms='read') AND subperms IS NULL)"; + if (! empty($subperms)) $whereforadd.=" OR (module='$module' AND perms='$perms' AND (subperms='lire' OR subperms='read'))"; + else if (! empty($perms)) $whereforadd.=" OR (module='$module' AND (perms='lire' OR perms='read') AND subperms IS NULL)"; } else { // On a pas demande un droit en particulier mais une liste de droits // sur la base d'un nom de module de de perms // Where pour la liste des droits a ajouter - if ($allmodule) $whereforadd="module='$allmodule'"; - if ($allperms) $whereforadd=" AND perms='$allperms'"; + if (! empty($allmodule)) $whereforadd="module='".$this->escape($allmodule)."'"; + if (! empty($allperms)) $whereforadd=" AND perms='".$this->escape($allperms)."'"; } // Ajout des droits trouves grace au critere whereforadd - if ($whereforadd) + if (! empty($whereforadd)) { //print "$module-$perms-$subperms"; $sql = "SELECT id"; @@ -381,7 +381,7 @@ class User extends CommonObject * @param string $allperms Retirer tous les droits du module allmodule, perms allperms * @return int > 0 if OK, < 0 if OK */ - function delrights($rid,$allmodule='',$allperms='') + function delrights($rid, $allmodule='', $allperms='') { global $conf; @@ -390,13 +390,13 @@ class User extends CommonObject $this->db->begin(); - if ($rid) + if (! empty($rid)) { // Si on a demande supression d'un droit en particulier, on recupere // les caracteristiques module, perms et subperms de ce droit. $sql = "SELECT module, perms, subperms"; $sql.= " FROM ".MAIN_DB_PREFIX."rights_def"; - $sql.= " WHERE id = '".$rid."'"; + $sql.= " WHERE id = '".$this->escape($rid)."'"; $sql.= " AND entity = ".$conf->entity; $result=$this->db->query($sql); @@ -412,7 +412,7 @@ class User extends CommonObject } // Where pour la liste des droits a supprimer - $wherefordel="id=".$rid; + $wherefordel="id=".$this->escape($rid); // Suppression des droits induits if ($subperms=='lire' || $subperms=='read') $wherefordel.=" OR (module='$module' AND perms='$perms' AND subperms IS NOT NULL)"; if ($perms=='lire' || $perms=='read') $wherefordel.=" OR (module='$module')"; @@ -420,12 +420,12 @@ class User extends CommonObject else { // On a demande suppression d'un droit sur la base d'un nom de module ou perms // Where pour la liste des droits a supprimer - if ($allmodule) $wherefordel="module='$allmodule'"; - if ($allperms) $wherefordel=" AND perms='$allperms'"; + if (! empty($allmodule)) $wherefordel="module='".$this->escape($allmodule)."'"; + if (! empty($allperms)) $wherefordel=" AND perms='".$this->escape($allperms)."'"; } // Suppression des droits selon critere defini dans wherefordel - if ($wherefordel) + if (! empty($wherefordel)) { //print "$module-$perms-$subperms"; $sql = "SELECT id"; diff --git a/htdocs/user/class/usergroup.class.php b/htdocs/user/class/usergroup.class.php index 5903d7e35c3..e98fb026a23 100644 --- a/htdocs/user/class/usergroup.class.php +++ b/htdocs/user/class/usergroup.class.php @@ -229,7 +229,7 @@ class UserGroup extends CommonObject * @param string $allperms Ajouter tous les droits du module allmodule, perms allperms * @return int > 0 if OK, < 0 if KO */ - function addrights($rid,$allmodule='',$allperms='') + function addrights($rid, $allmodule='', $allperms='') { global $conf; @@ -239,13 +239,13 @@ class UserGroup extends CommonObject $this->db->begin(); - if ($rid) + if (! empty($rid)) { // Si on a demande ajout d'un droit en particulier, on recupere // les caracteristiques (module, perms et subperms) de ce droit. $sql = "SELECT module, perms, subperms"; $sql.= " FROM ".MAIN_DB_PREFIX."rights_def"; - $sql.= " WHERE id = '".$rid."'"; + $sql.= " WHERE id = '".$this->escape($rid)."'"; $sql.= " AND entity = ".$conf->entity; $result=$this->db->query($sql); @@ -261,7 +261,7 @@ class UserGroup extends CommonObject } // Where pour la liste des droits a ajouter - $whereforadd="id=".$rid; + $whereforadd="id=".$this->escape($rid); // Ajout des droits induits if ($subperms) $whereforadd.=" OR (module='$module' AND perms='$perms' AND (subperms='lire' OR subperms='read'))"; else if ($perms) $whereforadd.=" OR (module='$module' AND (perms='lire' OR perms='read') AND subperms IS NULL)"; @@ -272,12 +272,12 @@ class UserGroup extends CommonObject } else { // Where pour la liste des droits a ajouter - if ($allmodule) $whereforadd="module='$allmodule'"; - if ($allperms) $whereforadd=" AND perms='$allperms'"; + if (! empty($allmodule)) $whereforadd="module='".$this->escape($allmodule)."'"; + if (! empty($allperms)) $whereforadd=" AND perms='".$this->escape($allperms)."'"; } // Ajout des droits de la liste whereforadd - if ($whereforadd) + if (! empty($whereforadd)) { //print "$module-$perms-$subperms"; $sql = "SELECT id"; @@ -330,7 +330,7 @@ class UserGroup extends CommonObject * @param string $allperms Retirer tous les droits du module allmodule, perms allperms * @return int > 0 if OK, < 0 if OK */ - function delrights($rid,$allmodule='',$allperms='') + function delrights($rid, $allmodule='', $allperms='') { global $conf; @@ -339,13 +339,13 @@ class UserGroup extends CommonObject $this->db->begin(); - if ($rid) + if (! empty($rid)) { // Si on a demande supression d'un droit en particulier, on recupere // les caracteristiques module, perms et subperms de ce droit. $sql = "SELECT module, perms, subperms"; $sql.= " FROM ".MAIN_DB_PREFIX."rights_def"; - $sql.= " WHERE id = '".$rid."'"; + $sql.= " WHERE id = '".$this->escape($rid)."'"; $sql.= " AND entity = ".$conf->entity; $result=$this->db->query($sql); @@ -361,7 +361,7 @@ class UserGroup extends CommonObject } // Where pour la liste des droits a supprimer - $wherefordel="id=".$rid; + $wherefordel="id=".$this->escape($rid); // Suppression des droits induits if ($subperms=='lire' || $subperms=='read') $wherefordel.=" OR (module='$module' AND perms='$perms' AND subperms IS NOT NULL)"; if ($perms=='lire' || $perms=='read') $wherefordel.=" OR (module='$module')"; @@ -372,12 +372,12 @@ class UserGroup extends CommonObject } else { // Where pour la liste des droits a supprimer - if ($allmodule) $wherefordel="module='$allmodule'"; - if ($allperms) $wherefordel=" AND perms='$allperms'"; + if (! empty($allmodule)) $wherefordel="module='".$this->escape($allmodule)."'"; + if (! empty($allperms)) $wherefordel=" AND perms='".$this->escape($allperms)."'"; } // Suppression des droits de la liste wherefordel - if ($wherefordel) + if (! empty($wherefordel)) { //print "$module-$perms-$subperms"; $sql = "SELECT id"; diff --git a/htdocs/user/group/perms.php b/htdocs/user/group/perms.php index 59fd1b32cb0..0aff65c7afc 100644 --- a/htdocs/user/group/perms.php +++ b/htdocs/user/group/perms.php @@ -33,9 +33,10 @@ $langs->load("users"); $langs->load("admin"); $id=GETPOST('id','int'); -$action=GETPOST("action"); -$confirm=GETPOST("confirm"); -$module=GETPOST("module"); +$action=GETPOST('action', 'alpha'); +$confirm=GETPOST('confirm', 'alpha'); +$module=GETPOST('module', 'alpha'); +$rights=GETPOST('rights', 'int'); // Defini si peux lire les permissions $canreadperms=($user->admin || $user->rights->user->user->lire); @@ -60,14 +61,14 @@ if ($action == 'addrights' && $caneditperms) { $editgroup = new Usergroup($db); $result=$editgroup->fetch($id); - if ($result > 0) $editgroup->addrights($_GET["rights"],$module); + if ($result > 0) $editgroup->addrights($rights, $module); } if ($action == 'delrights' && $caneditperms) { $editgroup = new Usergroup($db); $result=$editgroup->fetch($id); - if ($result > 0) $editgroup->delrights($_GET["rights"],$module); + if ($result > 0) $editgroup->delrights($rights, $module); } @@ -160,7 +161,7 @@ if ($id) { $sql.= " AND r.entity IN (0,".$conf->entity.")"; } - + $sql.= " AND ugr.fk_usergroup = ".$fgroup->id; $result=$db->query($sql); @@ -250,7 +251,7 @@ if ($id) $i = 0; $var = true; $oldmod = ''; - + $num = $db->num_rows($result); while ($i < $num) diff --git a/htdocs/user/perms.php b/htdocs/user/perms.php index 93eb28b4056..4e9aac8fee0 100644 --- a/htdocs/user/perms.php +++ b/htdocs/user/perms.php @@ -34,7 +34,8 @@ $langs->load("admin"); $id=GETPOST('id', 'int'); $action=GETPOST('action', 'alpha'); $confirm=GETPOST('confirm', 'alpha'); -$module=GETPOST('module'); +$module=GETPOST('module', 'alpha'); +$rights=GETPOST('rights', 'int'); if (! isset($id) || empty($id)) accessforbidden(); @@ -70,7 +71,7 @@ if ($action == 'addrights' && $caneditperms) { $edituser = new User($db); $edituser->fetch($id); - $edituser->addrights($_GET["rights"],$module); + $edituser->addrights($rights, $module); // Si on a touche a ses propres droits, on recharge if ($id == $user->id) @@ -84,7 +85,7 @@ if ($action == 'delrights' && $caneditperms) { $edituser = new User($db); $edituser->fetch($id); - $edituser->delrights($_GET["rights"],$module); + $edituser->delrights($rights, $module); // Si on a touche a ses propres droits, on recharge if ($id == $user->id)