Discussion :

[helpcomm]

Salut,

Pour le moment j'évite de trop utiliser les $_GET dans mon application PHP, parce que je manque de compétences dans la sécurisation de ces variables. Ce que je fais pour le moment, c'est passer juste des paramètres numériques (comme "test.php?ajout=1"). Et après je fais ce test avant de continuer le traitement:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

if (isset($_GET['ajout']) && ($_GET['ajout'] == 1) && is_numeric($_GET['ajout']))

Est ce que vous croyez que cela est suffisent?
Si dans le futur je suis obligé d'utiliser les variables string vous me suggérez quelle ressource pour être sure que c'est bien sécurisé?

Merci

[ABCIWEB]

C'est pas forcément non sécurisé de passer des string comme variable dans l'URL. Cela dépend surtout à quoi te sert cette variable.

Il faut faire en sorte à la réception de la variable que celle-ci, même si elle n'est pas ce qu'on attend, ne puisse pas nuire à ton script.

Ou si tu peux connaître à l'avance ce qu'elle peut contenir tu peux faire un tableau des valeurs autorisées :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
$tab = array('valeur1', 'valeur2', 'valeur3');
 
$get_var = isset($_GET['var']) && in_array($_GET['var'],$tab) ? $_GET['var'] : 'valeur par defaut';

[Finality]

Salut,

il n'y a pas de règle ultime de sécurité, mais plutôt des bonnes pratiques à prendre en compte :

1) Si ta variable est utilisée pour une requête SQL, penses à la traiter avec une fonction php afin d'enlever tout caractère dangereux (addslashes, ...).

2) Si la variable sert à faire une sélection sur un ID, il faut bien tester que l'enregistrement correspondant existe

3) Comme l'a dis ABCIWEB, si pour certaines variables tu imposes des valeurs spécifiques, il faut bien tester leur validité.

De toute façon, que tu passes tes variables en GET ou en POST, un utilisateur pourra toujours changer leur valeur. Il faut donc dans tous les cas mettre en place ce côté contrôle des données

[nextdev]

A ta place, je ferais dans ce genre :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
$ajout = 0;
if(isset($_GET['ajout'])) { $ajout = intval($_GET['ajout']); }
if($ajout === 1) {....

C'est rapide, lisible, passé en int pour du controle directe avec le strictement égale, et ça suffira
Ce que tu as fait est bon aussi.

[ABCIWEB]

1) Si ta variable est utilisée pour une requête SQL, penses à la traiter avec une fonction php afin d'enlever tout caractère dangereux (addslashes, ...).

Petite précision,

...si ta variable est utilisée pour une requête SQL, penses à la traiter impérativement avec mysql_real_escape_string si tu utilise mysql ou mysqli ou fais une requête préparée si tu utilise PDO.

[ABCIWEB]

A ta place, je ferais dans ce genre :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
$ajout = 0;
if(isset($_GET['ajout'])) { $ajout = intval($_GET['ajout']); }
if($ajout === 1) {....

C'est rapide, lisible, passé en int pour du controle directe avec le strictement égale, et ça suffira
Ce que tu as fait est bon aussi.

On peut le faire en une ligne avec les opérateurs ternaires

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

$ajout = isset($_GET['ajout']) ? intval($_GET['ajout']) : 0;

Concernant le comparateur strict "===" qui compare le type de la variable en plus de sa valeur, dans ton exemple ce n'est pas nécessaire puisque tu castes ta variable en entier et que la valeur par défaut est également un entier, on est donc certain du type

[nextdev]

jte pari que ton code est moins rapide, également avec un simple ==
Et pour encore meilleur perf, tu conserves que la variable $_GET['ajout'], et avec test empty

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
if(empty($_GET['ajout'])) { $_GET['ajout']=0; }else{ $_GET['ajout']=intval($_GET['ajout']); }
if($_GET['ajout'] === 1) {....

[transgohan]

Attention au comportement de intval() cependant.

Code php :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
if( intval("5bis") == 5 )
  echo "Ce test est vrai !";

[ABCIWEB]

jte pari que ton code est moins rapide, également avec un simple ==
Et pour encore meilleur perf, tu conserves que la variable $_GET['ajout'], et avec test empty

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
if(empty($_GET['ajout'])) { $_GET['ajout']=0; }else{ $_GET['ajout']=intval($_GET['ajout']); }
if($_GET['ajout'] === 1) {....

C'est pas impossible mais c'est tellement infime que je préfère la lisibilité. De toutes façons c'est pas à ce niveau là qu'on optimise un script

[pi-2r]

Bonjour,

voici une idée de sécurisation des variables GET

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
$perso=mysql_real_escape_string(htmlspecialchars($_GET['perso']));
if(!empty($perso) && is_numeric($perso) && $perso>0 )
{
	$sql =mysql_query("SELECT * 
				      FROM table
				      WHERE  perso='$perso'
				    ") or die(mysql_error());
	$res=mysql_fetch_array($sql);
}
else
{
header('Location: index.php');
}

[nextdev]

Oui, c'est sur que niveau performance, on va pas gagner grand chose si il y a qu'un seul test , donc selon le script, voir ce niveau la aussi.

Bon rappel transgohan

Et pour empty, attention si la valeur 0 peu être envoyée.

Il y a quelqu'un qui a pas l'air d'aimer nos contrôles, lui il les fait sûrement avec un simple if($_GET['ajout'] == 1) {... et en display_errors off
Je signale que ces tests sont fait pour contrôler des entiers seulement...



Édition :
Donc après nos script contenant intval et ===, la variable ajout est prête à être utilisé dans une requête en toute sécurité.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

SELECT * FROM table WHERE entier=$ajout

[pi-2r]

pour ma part les 0 ne sont jamais passé en GET, car j'appel l'ID du produit, d'une personne,.... d'ou ma vérification par empty.

Ensuite, je vérifie bien que la valeur est numérique car par une injetcion blind sql, il est possible de contourner une vérification trop simple (ex: dossier.php?id=1+union+select+1,@@version).

La dernière vérification est présente pour vérifier que l'ID est toujours supérieur à 0 (ce qui est toujours le cas dans un table), car pour détecter des injection sql il suffit parfois de mettre un "-" devant un chiffre(ex: dossier.php?id=-1)

[stealth35]

utilisez filter_input, c'est fait pour

[ABCIWEB]

Bonjour,

voici une idée de sécurisation des variables GET

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
$perso=mysql_real_escape_string(htmlspecialchars($_GET['perso']));
if(!empty($perso) && is_numeric($perso) && $perso>0 )
{
	$sql =mysql_query("SELECT * 
				      FROM table
				      WHERE  perso='$perso'
				    ") or die(mysql_error());
	$res=mysql_fetch_array($sql);
}
else
{
header('Location: index.php');
}

On emploie pas htmlspecialchars autrement que pour sécuriser une variable lors de son affichage (sauf exception).

Pour la petite histoire, y'a toute une flopée de scripts "mal conçus" qui enregistrent les données en bdd avec htmlspecialchars (ou htmlentities). C'est un exemple à ne pas suivre car on encombre les tables pour rien avec ces entités html, on rend par la même occasion la table non exportable dans un format standard, on peut avoir de sérieux problèmes pour faire des recherches sur la table et j'en oublie...

Aussi faudrait faire un isset($_GET['perso']) avant de définir $perso. En toute logique on commence par définir la variable et on lui applique ensuite des fonctions.
Pour faire le "même" code que tu as écrit je verrais plutôt ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
$perso = isset($_GET['perso']) &&  is_numeric($_GET['perso']) && $_GET['perso'] > 0 ? intval($_GET['perso']) ; null;
 
if (isset($perso))
{
        $perso=mysql_real_escape_string($perso);
 
	$sql =mysql_query("SELECT * 
				      FROM table
				      WHERE  perso='$perso'
				    ") or die(mysql_error());
	$res=mysql_fetch_array($sql);
}

Bon là, la fonction mysql_real_escape_string est inutile puisque pour sécuriser un peu plus la variable (si on attend un entier) j'ai mis intval($_GET['perso']) et que mysql_real_escape_string ne fait rien sur un entier. Mais c'est une bonne habitude à prendre que d'utiliser cette fonction quand on passe une variable dans une requête.
L'avantage de cette configuration est qu'il n'y a qu'à changer la première ligne si $perso est d'une autre nature. Exemple pour caster en string si on attend une chaine de caractère non vide.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

$perso = isset($_GET['perso']) &&  trim($_GET['perso']) != '' ? (string) $_GET['perso'] : null;

La suite du code reste inchangé, puisque l'on a concentré le contrôle sur la première ligne en définissant la variable. Bon ce n'est pas toujours applicable mais dans la plupart des cas ça améliore la lisibilité et donc la maintenance

[helpcomm]

Est ce que tu peux nous expliquer une autre façon de gérer les apostrophes? Si l'utilisateur entre "l'oiseau" et on ne n'utilise pas htmlentities la requete ne passera pas j'imagine.
J'utilise souvent:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

$str = htmlentities(mysql_real_escape_string(trim($str)), ENT_NOQUOTES, "UTF-8");

avant d'insérer dans la BD. Si c'est pas une bonne méthode, est ce qu'il y a une alternative?

[Eric2a]

Salut,

Pour la gestion des apostrophes, faire un petit tour ici

[ABCIWEB]

Est ce que tu peux nous expliquer une autre façon de gérer les apostrophes? Si l'utilisateur entre "l'oiseau" et on ne n'utilise pas htmlentities la requete ne passera pas j'imagine.
J'utilise souvent:
$str = htmlentities(mysql_real_escape_string(trim($str)), ENT_NOQUOTES, "UTF-8");
avant d'insérer dans la BD. Si c'est pas une bonne méthode, est ce qu'il y a une alternative?

Il n'est dit nulle part dans le manuel qu'on utilise htmlentities ou htmlspecialchars pour gérer des apostrophes et non plus pour insérer les données en bdd. Faut te référer au manuel ! il suffit de faire :
$str = mysql_real_escape_string(trim($str));
(le trim étant en option)
Cela est valable pour mysql et pour mysqli. Avec PDO c'est différent.

Ensuite comme indiqué dans le lien donné par Eric2a, certains serveurs peuvent encore avoir la fonction get_magic_quotes_gpc() activée. Pour un maximum de portabilité du script il est utile d'en tenir compte et donc de supprimer les antislash insérés automatiquement (si get_magic_quotes_gpc() est activé) dans les variables gpc soit get, post, cookie. Sur ces variables on peut donc appliquer une petite fonction qui supprimera éventuellement les antislash avec stripslashes en testant la configuration get_magic_quotes_gpc() du serveur.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
function Verif_magicquotes ($chaine)// fonction qui applique stripslashes si get_magic_quotes_gpc() est activé
    {
	if (get_magic_quotes_gpc()) $chaine = stripslashes($chaine);
 
	return $chaine;
    }

Et donc le code que je donnais en exemple plus haut devient :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
function Verif_magicquotes ($chaine)// fonction qui applique stripslashes si get_magic_quotes_gpc() est activé
    {
	if (get_magic_quotes_gpc()) $chaine = stripslashes($chaine);
 
	return $chaine;
    }
 
 
$perso = isset($_GET['perso']) &&  trim($_GET['perso']) != '' ? (string) Verif_magicquotes ($_GET['perso']) : null;
 
if (isset($perso))
{
        $perso=mysql_real_escape_string($perso);
 
	$sql =mysql_query("SELECT * 
				      FROM table
				      WHERE  perso='$perso'
				    ") or die(mysql_error());
	$res=mysql_fetch_array($sql);
//...
}

C'est un exemple de code qui passe partout quelque soit la configuration du serveur. Bon c'est un peu rageant d'appliquer une fonction qui ne servira plus à court/moyen terme puisque get_magic_quotes_gpc est obsolète depuis php5.3, mais en même temps cela ne gêne pas.

Remarques que si tu ne vois nulle part htmlspecialchars ni htmlentities c'est qu'il n'y en a pas besoin dans ce contexte. Encore une fois ces fonctions ne servent que pour l'affichage : echo htmlspecialchars($_GET['perso'])
(et donc sauf exception, elles sont précédées d'un "echo")

Dernière chose il est généralement préférable (toujours pour l'affichage) d'utiliser htmlspecialchars plutôt que htmlentities. Simplement parce que si tu utilise l'utf-8 tu n'auras pas besoin de spécifier l'encodage avec htmlspecialchars et tu peux donc faire simplement echo htmlspecialchars($perso)

[helpcomm]

Salut,

Merci beaucoup pour vos commentaires très enrichissants. Le nombre de réponses montre la richesse du sujet, et le besoin d'y voir plus clair pour beaucoup de programmeurs. Sur internet j'ai trouvé des exemples effrayants par leur complexité; ils utilisent les expressions régulières pour écarter les cas où c'est une URL qui est soumise en paramètres ...
Dans un essai de synthèse qui serait utile pour les visiteurs du site et pour m'assurer que j'ai bien compris aussi, je vous transmet un exemple qui traite uniquement le cas où la variable $_GET va figurer dans la requête sql et pour alléger le texte je ne considère pas le cas de magic quotes:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
$tab = array('valeur1', 'valeur2', 'valeur3');//les cas à traiter
$get_var = trim($_GET['var']);
$get_var = isset($_GET['var']) && in_array($get_var,$tab) ? $get_var : null;
 
//cas string
if ($get_var == 'valeur1') {
	$get_var = mysql_real_escape_string($get_var);
	//exécuter la requête contenant $get_var
}
 
//cas entier avec des cas connus d'avance à traiter ici 5 par exemple
if(!empty($get_var) && is_numeric($get_var) && $get_var == 5 ) {
	//executer requete contenant $get_var 
}
 
//cas entier sans avoir des cas connus d'avance
if(!empty($get_var) && is_numeric($get_var) && $get_var > 0 ) {
	//executer requete contenant $get_var 
}

[ABCIWEB]

Attention tu fais $get_var = trim($_GET['var']); sans savoir si $_GET['var'] est définie. De plus cette ligne ne sert à rien. Passe directement à la deuxième en faisant :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
 
$tab = array('valeur1', 'valeur2', 'valeur3');
 
$get_var = isset($_GET['var']) && in_array($_GET['var'],$tab) ? $_GET['var'] : null;

Essaies de faire le code le plus simple et le plus élémentaire.

Pour tester l'existence d'une variable on utilise isset, ou !empty mais !empty est plus exigeant puisqu'il faut aussi que la variable soit différente de '', de 0 et de FALSE.

dans

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

if(!empty($get_var) && is_numeric($get_var) && $get_var == 5 ) {

tu dis textuellement : si la variable existe et est différente de '' de 0 de FALSE (!empty) qu'elle est numérique (is_numeric) et qu'elle est égale à 5...
cela revient au même et c'est plus simple de dire : si la variable existe (isset) qu'elle est numérique (is_numeric) et qu'elle est égale à 5.
donc

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

if(isset($get_var) && is_numeric($get_var) && $get_var == 5 )

Ce que tu fais revient au même mais il est utile de ne faire que le code minimum indispensable, comme ça quand tu relis plus tard, tu peux te dire que si c'est écrit c'est que tu avais une bonne raison de l'écrire.

[stealth35]

c'est :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

filter_input(INPUT_GET, 'var', FILTER_VALIDATE_INT, array('options' => array('min_range' => 1)));