Skip to content

Commit 63820ab

Browse files
committed
Security: A lot of security fixes
1 parent 295745f commit 63820ab

File tree

10 files changed

+74
-70
lines changed

10 files changed

+74
-70
lines changed

Diff for: htdocs/lib/databases/mssql.lib.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -518,8 +518,8 @@ function order($sortfield=0,$sortorder=0)
518518
if (! $return) $return.=' ORDER BY ';
519519
else $return.=',';
520520

521-
$return.=$val;
522-
if ($sortorder) $return.=' '.$sortorder;
521+
$return.=preg_replace('/[^0-9a-z_\.]/i','',$val);
522+
if ($sortorder) $return.=' '.preg_replace('/[^0-9a-z]/i','',$sortorder);
523523
}
524524
return $return;
525525
}

Diff for: htdocs/lib/databases/mysql.lib.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ function plimit($limit=0,$offset=0)
494494

495495
/**
496496
* Define sort criteria of request
497+
*
497498
* @param sortfield List of sort fields
498499
* @param sortorder Sort order
499500
* @return string String to provide syntax of a sort sql string
@@ -510,8 +511,8 @@ function order($sortfield=0,$sortorder=0)
510511
if (! $return) $return.=' ORDER BY ';
511512
else $return.=',';
512513

513-
$return.=$val;
514-
if ($sortorder) $return.=' '.$sortorder;
514+
$return.=preg_replace('/[^0-9a-z_\.]/i','',$val);
515+
if ($sortorder) $return.=' '.preg_replace('/[^0-9a-z]/i','',$sortorder);
515516
}
516517
return $return;
517518
}

Diff for: htdocs/lib/databases/mysqli.lib.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,8 @@ function order($sortfield=0,$sortorder=0)
524524
if (! $return) $return.=' ORDER BY ';
525525
else $return.=',';
526526

527-
$return.=$val;
528-
if ($sortorder) $return.=' '.$sortorder;
527+
$return.=preg_replace('/[^0-9a-z_\.]/i','',$val);
528+
if ($sortorder) $return.=' '.preg_replace('/[^0-9a-z]/i','',$sortorder);
529529
}
530530
return $return;
531531
}

Diff for: htdocs/lib/databases/pgsql.lib.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -666,8 +666,8 @@ function order($sortfield=0,$sortorder=0)
666666
if (! $return) $return.=' ORDER BY ';
667667
else $return.=',';
668668

669-
$return.=$val;
670-
if ($sortorder) $return.=' '.$sortorder;
669+
$return.=preg_replace('/[^0-9a-z_\.]/i','',$val);
670+
if ($sortorder) $return.=' '.preg_replace('/[^0-9a-z]/i','',$sortorder);
671671
}
672672
return $return;
673673
}

Diff for: htdocs/user/fiche.php

+37-36
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@
3535
if ($conf->ldap->enabled) require_once(DOL_DOCUMENT_ROOT."/lib/ldap.class.php");
3636
if ($conf->adherent->enabled) require_once(DOL_DOCUMENT_ROOT."/adherents/class/adherent.class.php");
3737

38+
$id=GETPOST('id','int');
39+
$action=GETPOST("action");
40+
$group=GETPOST("group","int",3);
41+
$confirm=GETPOST("confirm");
42+
3843
// Define value to know what current user can do on users
3944
$canadduser=($user->admin || $user->rights->user->user->creer);
4045
$canreaduser=($user->admin || $user->rights->user->user->lire);
@@ -48,26 +53,22 @@
4853
$caneditgroup=($user->admin || $user->rights->user->group_advance->write);
4954
}
5055
// Define value to know what current user can do on properties of edited user
51-
if ($_GET["id"])
56+
if ($id)
5257
{
53-
// $user est le user qui edite, $_GET["id"] est l'id de l'utilisateur edite
54-
$caneditfield=( (($user->id == $_GET["id"]) && $user->rights->user->self->creer)
55-
|| (($user->id != $_GET["id"]) && $user->rights->user->user->creer) );
56-
$caneditpassword=( (($user->id == $_GET["id"]) && $user->rights->user->self->password)
57-
|| (($user->id != $_GET["id"]) && $user->rights->user->user->password) );
58+
// $user est le user qui edite, $id est l'id de l'utilisateur edite
59+
$caneditfield=( (($user->id == $id) && $user->rights->user->self->creer)
60+
|| (($user->id != $id) && $user->rights->user->user->creer) );
61+
$caneditpassword=( (($user->id == $id) && $user->rights->user->self->password)
62+
|| (($user->id != $id) && $user->rights->user->user->password) );
5863
}
5964

60-
$action=GETPOST("action");
61-
$group=GETPOST("group","int",3);
62-
$confirm=GETPOST("confirm");
63-
6465
// Security check
6566
$socid=0;
6667
if ($user->societe_id > 0) $socid = $user->societe_id;
6768
$feature2='user';
68-
if ($user->id == $_GET["id"]) { $feature2=''; $canreaduser=1; } // A user can always read its own card
69-
$result = restrictedArea($user, 'user', $_GET["id"], '', $feature2);
70-
if ($user->id <> $_GET["id"] && ! $canreaduser) accessforbidden();
69+
if ($user->id == $id) { $feature2=''; $canreaduser=1; } // A user can always read its own card
70+
$result = restrictedArea($user, 'user', $id, '', $feature2);
71+
if ($user->id <> $id && ! $canreaduser) accessforbidden();
7172

7273
$langs->load("users");
7374
$langs->load("companies");
@@ -82,36 +83,36 @@
8283
if ($_GET["subaction"] == 'addrights' && $canedituser)
8384
{
8485
$edituser = new User($db);
85-
$edituser->fetch($_GET["id"]);
86+
$edituser->fetch($id);
8687
$edituser->addrights($_GET["rights"]);
8788
}
8889

8990
if ($_GET["subaction"] == 'delrights' && $canedituser)
9091
{
9192
$edituser = new User($db);
92-
$edituser->fetch($_GET["id"]);
93+
$edituser->fetch($id);
9394
$edituser->delrights($_GET["rights"]);
9495
}
9596

9697
if ($action == 'confirm_disable' && $confirm == "yes" && $candisableuser)
9798
{
98-
if ($_GET["id"] <> $user->id)
99+
if ($id <> $user->id)
99100
{
100101
$edituser = new User($db);
101-
$edituser->fetch($_GET["id"]);
102+
$edituser->fetch($id);
102103
$edituser->setstatus(0);
103-
Header("Location: ".DOL_URL_ROOT.'/user/fiche.php?id='.$_GET["id"]);
104+
Header("Location: ".DOL_URL_ROOT.'/user/fiche.php?id='.$id);
104105
exit;
105106
}
106107
}
107108
if ($action == 'confirm_enable' && $confirm == "yes" && $candisableuser)
108109
{
109-
if ($_GET["id"] <> $user->id)
110+
if ($id <> $user->id)
110111
{
111112
$message='';
112113

113114
$edituser = new User($db);
114-
$edituser->fetch($_GET["id"]);
115+
$edituser->fetch($id);
115116

116117
if (!empty($conf->file->main_limit_users))
117118
{
@@ -125,18 +126,18 @@
125126
if (! $message)
126127
{
127128
$edituser->setstatus(1);
128-
Header("Location: ".DOL_URL_ROOT.'/user/fiche.php?id='.$_GET["id"]);
129+
Header("Location: ".DOL_URL_ROOT.'/user/fiche.php?id='.$id);
129130
exit;
130131
}
131132
}
132133
}
133134

134135
if ($action == 'confirm_delete' && $confirm == "yes" && $candisableuser)
135136
{
136-
if ($_GET["id"] <> $user->id)
137+
if ($id <> $user->id)
137138
{
138139
$edituser = new User($db);
139-
$edituser->id=$_GET["id"];
140+
$edituser->id=$id;
140141
$result = $edituser->delete();
141142
if ($result < 0)
142143
{
@@ -232,13 +233,13 @@
232233
$editgroup->oldcopy=dol_clone($editgroup);
233234

234235
$edituser = new User($db);
235-
$edituser->fetch($_GET["id"]);
236+
$edituser->fetch($id);
236237
if ($action == 'addgroup') $edituser->SetInGroup($group,GETPOST('entity'));
237238
if ($action == 'removegroup') $edituser->RemoveFromGroup($group,GETPOST('entity'));
238239

239240
if ($result > 0)
240241
{
241-
header("Location: fiche.php?id=".$_GET["id"]);
242+
header("Location: fiche.php?id=".$id);
242243
exit;
243244
}
244245
else
@@ -271,7 +272,7 @@
271272
{
272273
$db->begin();
273274
$edituser = new User($db);
274-
$edituser->fetch($_GET["id"]);
275+
$edituser->fetch($id);
275276

276277
$edituser->oldcopy=dol_clone($edituser);
277278

@@ -360,7 +361,7 @@
360361
else if ($caneditpassword) // Case we can edit only password
361362
{
362363
$edituser = new User($db);
363-
$edituser->fetch($_GET["id"]);
364+
$edituser->fetch($id);
364365

365366
$edituser->oldcopy=dol_clone($edituser);
366367

@@ -377,7 +378,7 @@
377378
|| ($action == 'confirm_passwordsend' && $confirm == 'yes')) && $caneditpassword)
378379
{
379380
$edituser = new User($db);
380-
$edituser->fetch($_GET["id"]);
381+
$edituser->fetch($id);
381382

382383
$newpassword=$edituser->setPassword($user,'');
383384
if ($newpassword < 0)
@@ -800,10 +801,10 @@
800801
/* */
801802
/* ************************************************************************** */
802803

803-
if ($_GET["id"])
804+
if ($id)
804805
{
805806
$fuser = new User($db);
806-
$fuser->fetch($_GET["id"]);
807+
$fuser->fetch($id);
807808

808809
// Connexion ldap
809810
// pour recuperer passDoNotExpire et userChangePassNextLogon
@@ -1169,13 +1170,13 @@
11691170
// Si on a un gestionnaire de generation de mot de passe actif
11701171
if ($conf->global->USER_PASSWORD_GENERATED != 'none')
11711172
{
1172-
if (($user->id != $_GET["id"] && $caneditpassword) && $fuser->login && !$fuser->ldap_sid &&
1173+
if (($user->id != $id && $caneditpassword) && $fuser->login && !$fuser->ldap_sid &&
11731174
(empty($conf->multicompany->enabled) || ($fuser->entity == $conf->entity)))
11741175
{
11751176
print '<a class="butAction" href="fiche.php?id='.$fuser->id.'&amp;action=password">'.$langs->trans("ReinitPassword").'</a>';
11761177
}
11771178

1178-
if (($user->id != $_GET["id"] && $caneditpassword) && $fuser->login && !$fuser->ldap_sid &&
1179+
if (($user->id != $id && $caneditpassword) && $fuser->login && !$fuser->ldap_sid &&
11791180
(empty($conf->multicompany->enabled) || ($fuser->entity == $conf->entity)) )
11801181
{
11811182
if ($fuser->email) print '<a class="butAction" href="fiche.php?id='.$fuser->id.'&amp;action=passwordsend">'.$langs->trans("SendNewPassword").'</a>';
@@ -1184,19 +1185,19 @@
11841185
}
11851186

11861187
// Activer
1187-
if ($user->id <> $_GET["id"] && $candisableuser && $fuser->statut == 0 &&
1188+
if ($user->id <> $id && $candisableuser && $fuser->statut == 0 &&
11881189
(empty($conf->multicompany->enabled) || ($fuser->entity == $conf->entity)) )
11891190
{
11901191
print '<a class="butAction" href="fiche.php?id='.$fuser->id.'&amp;action=enable">'.$langs->trans("Reactivate").'</a>';
11911192
}
11921193
// Desactiver
1193-
if ($user->id <> $_GET["id"] && $candisableuser && $fuser->statut == 1 &&
1194+
if ($user->id <> $id && $candisableuser && $fuser->statut == 1 &&
11941195
(empty($conf->multicompany->enabled) || ($fuser->entity == $conf->entity)) )
11951196
{
11961197
print '<a class="butActionDelete" href="fiche.php?action=disable&amp;id='.$fuser->id.'">'.$langs->trans("DisableUser").'</a>';
11971198
}
11981199
// Delete
1199-
if ($user->id <> $_GET["id"] && $candisableuser &&
1200+
if ($user->id <> $id && $candisableuser &&
12001201
(empty($conf->multicompany->enabled) || ($fuser->entity == $conf->entity)) )
12011202
{
12021203
print '<a class="butActionDelete" href="fiche.php?action=delete&amp;id='.$fuser->id.'">'.$langs->trans("DeleteUser").'</a>';
@@ -1232,7 +1233,7 @@
12321233
if ($caneditgroup)
12331234
{
12341235
$form = new Form($db);
1235-
print '<form action="fiche.php?id='.$_GET["id"].'" method="post">'."\n";
1236+
print '<form action="fiche.php?id='.$id.'" method="post">'."\n";
12361237
print '<input type="hidden" name="token" value="'.$_SESSION['newtoken'].'">';
12371238
print '<input type="hidden" name="action" value="addgroup">';
12381239
print '<input type="hidden" name="entity" value="'.$conf->entity.'">';

Diff for: htdocs/user/index.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
$socid=0;
3636
if ($user->societe_id > 0) $socid = $user->societe_id;
3737

38-
$sall=isset($_GET["sall"])?$_GET["sall"]:$_POST["sall"];
38+
$sall=GETPOST("sall");
3939

4040
$sortfield = GETPOST("sortfield",'alpha');
4141
$sortorder = GETPOST("sortorder",'alpha');
@@ -51,6 +51,7 @@
5151
$userstatic=new User($db);
5252
$companystatic = new Societe($db);
5353

54+
5455
/*
5556
* View
5657
*/
@@ -73,9 +74,8 @@
7374
{
7475
$sql.= " AND (u.login like '%".$_POST["search_user"]."%' OR u.name like '%".$_POST["search_user"]."%' OR u.firstname like '%".$_POST["search_user"]."%')";
7576
}
76-
if ($sall) $sql.= " AND (u.login like '%".$sall."%' OR u.name like '%".$sall."%' OR u.firstname like '%".$sall."%' OR u.email like '%".$sall."%' OR u.note like '%".$sall."%')";
77-
if ($sortfield) $sql.=" ORDER BY $sortfield $sortorder";
78-
77+
if ($sall) $sql.= " AND (u.login like '%".$db->escape($sall)."%' OR u.name like '%".$db->escape($sall)."%' OR u.firstname like '%".$db->escape($sall)."%' OR u.email like '%".$db->escape($sall)."%' OR u.note like '%".$db->escape($sall)."%')";
78+
$sql.=$db->order($sortfield,$sortorder);
7979
$result = $db->query($sql);
8080
if ($result)
8181
{

Diff for: htdocs/user/info.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
$langs->load("users");
3131

3232
// Security check
33-
$id = isset($_GET["id"])?$_GET["id"]:'';
33+
$id = GETPOST('id','int');
3434
$fuser = new User($db);
3535
$fuser->fetch($id);
3636

Diff for: htdocs/user/note.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
require_once(DOL_DOCUMENT_ROOT.'/lib/usergroups.lib.php');
2828
require_once(DOL_DOCUMENT_ROOT.'/user/class/user.class.php');
2929

30-
$action=isset($_GET["action"])?$_GET["action"]:(isset($_POST["action"])?$_POST["action"]:"");
31-
$id=isset($_GET["id"])?$_GET["id"]:(isset($_POST["id"])?$_POST["id"]:"");
30+
$action=GETPOST('action');
31+
$id=GETPOST('id','int');
3232

3333
$langs->load("companies");
3434
$langs->load("members");

Diff for: htdocs/user/param_ihm.php

+9-8
Original file line numberDiff line numberDiff line change
@@ -33,30 +33,31 @@
3333
$langs->load("users");
3434
$langs->load("languages");
3535

36+
$id=GETPOST('id','int');
37+
3638
// Defini si peux lire/modifier permisssions
3739
$canreaduser=($user->admin || $user->rights->user->user->lire);
3840

39-
if ($_REQUEST["id"])
41+
if ($id)
4042
{
41-
// $user est le user qui edite, $_REQUEST["id"] est l'id de l'utilisateur edite
42-
$caneditfield=( (($user->id == $_REQUEST["id"]) && $user->rights->user->self->creer)
43-
|| (($user->id != $_REQUEST["id"]) && $user->rights->user->user->creer));
43+
// $user est le user qui edite, $id est l'id de l'utilisateur edite
44+
$caneditfield=( (($user->id == $id) && $user->rights->user->self->creer)
45+
|| (($user->id != $id) && $user->rights->user->user->creer));
4446
}
4547

4648
// Security check
4749
$socid=0;
4850
if ($user->societe_id > 0) $socid = $user->societe_id;
4951
$feature2 = (($socid && $user->rights->user->self->creer)?'':'user');
50-
if ($user->id == $_REQUEST["id"]) // A user can always read its own card
52+
if ($user->id == $id) // A user can always read its own card
5153
{
5254
$feature2='';
5355
$canreaduser=1;
5456
}
55-
$result = restrictedArea($user, 'user', $_REQUEST["id"], '', $feature2);
56-
if ($user->id <> $_REQUEST["id"] && ! $canreaduser) accessforbidden();
57+
$result = restrictedArea($user, 'user', $id, '', $feature2);
58+
if ($user->id <> $id && ! $canreaduser) accessforbidden();
5759

5860

59-
$id=! empty($_GET["id"])?$_GET["id"]:$_POST["id"];
6061
$dirtop = "../includes/menus/standard";
6162
$dirleft = "../includes/menus/standard";
6263

0 commit comments

Comments
 (0)