[Sécurité] Nettoyage de données [Résolu]

[illimus]

Bonjour,

J'aurais voulu connaitre votre avis sur ma fonction.
Est-ce inutile, trop lourd ,...., génial ;-)

C'est une fonction qui controle les variables des tableaux de POST et GET
mes variables sont notées avec des prefixes : int_, id_,...

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
 function clean_data(){
    foreach($_REQUEST as $key=>$var) {
        if(!is_array($var)){
            // recuperation du nom de la variable
            $type_var = explode("_", $key);
            switch ($type_var[0]){
                // entier 
                case "int":
                    $clean_var = sanitize($var, INT);
                break;
 
                // entier 
                case "id":
                    $clean_var = sanitize($var, INT);
                break;
 
                // chaine de caratère 
                case "str":
                    $clean_var = sanitize($var, HTML);
                break;        
 
                // entier floatant
                case "float":
                    $clean_var = sanitize($var, FLOAT);
                break;
 
                // si non classé = mode paranoïa ;-)
                default:
                    $clean_var = sanitize($var, PARANOID);
            }
        }
 
        $clean_var = sanitize($clean_var, SQL);
 
        if(isset($_POST[$key])){
            $_POST[$key]=$clean_var;
            echo "Post";
        }
 
        if(isset($_GET[$key])){
            $_GET[$key]=$clean_var;
            echo "get";
        }
    }    
}

J'utilise une bibliothèque pour nettoyer les données
* Open Web Application Security Project
* (http://www.owasp.org)

[Hervé Saladin]

Salut,
inutile ? certainement pas si tu as un minimum de sécurité à assurer.
trop lourd ? difficile à dire, faudrait voir le code de la fonction sanitize() (j'avoue que j'ai pas eu le courage de chercher), et puis tout dépend des capacités du serveur, de la charge qu'il supporte etc ...
génial ? euh, non quand même pas
D'une façon générale, c'est cela ou quelque chose de très similaire que tout développeur web devrait faire pour toutes ses applis PHP, ASP etc : vérifier les parametres GET et POST, les cookies etc ... afin d'éviter les mauvaises surprises.

[illimus]

Ok merci, ca me rassure.

Pour la fonction si tu veux je te l'envoie demain, mais grosso modo c'est de l'expression regulière.

Pour les attaques Xss y a t'il d'autre données transmises qu'il faudrait filtrées ?, les sessions et cookies sont-ils une "menace" aussi ?

[Hervé Saladin]

Toute donnée ne provenant pas de tes scripts ou de ta base de donnée doit être vérifiée.
Pour être le plus tranquille possible, il faut échapper les caractères spéciaux (guillemets, quotes, & cie), et aussi proscrire les mots-clés SQL (tous), et plus généralement tout ce qui n'a rien à faire dans la requete HTTP.
Si tu veux être paranoïaque (ce qui n'est pas forcément incensé), le mieux est encore de faire une expression réguliere très restrictive pour chaque parametre (ou chaque type de parametre) à laquelle le parametre en question doit correspondre et si il ne correspond pas, tu ne le traite pas ou tu donne un message d'erreur à l'utilisateur.

Typiquement : tous les parametres passés dans la requete HTTP (données de formulaire, cookies) doivent être passés au crible.
Donc OUI les cookies doivet êtres vérifiés, car ils sont transmis dans la requête HTTP au serveur par le navigateur client, au même titre qu'un paramètre GET ou POST, donc un hacker qui modifierait lui-même les cookies sur son ordi pourrait exploiter exactement les mêmes vulnérabilités.

Par contre, les variables de session ne présentent pas de risque en elles-mêmes car elles ne sont crées, stockées et traitées que sur le serveur, le client n'a aucune prise dessus.

Enfin, saches qu'il existe des outils destinés à verifier la sécurité de tes scripts CGI (PHP, ASP, Perl, Java etc ...).
Comme tous les outils de sécurité, c'est à double tranchant (tout dépend l'objectif pour lequel on s'en sert), aussi je n'en citerais pas ici, mais je te fais confiance pour trouver toi même et en faire bon usage

Bon, je sais que là j'ai l'air d'un parano qui voit des hackers et des failles de sécurité partout, mais je peux t'assurer que c'est pas du luxe d'être prudent. J'en ai eu la douloureuse experience y'a pas très longtemps : hack d'un serveur, avec installation de portes dérobées, défacement d'un site, et utilisation du serveur comme relai pour des attaques (D?)DOS .... tout ça à cause d'une appli ASP vulnérable aux injections SQL (pourtant cette appli avait été développée par une SSII serieuse, pas par le petit stagiaire de 1ere année).
Bref, une bonne semaine de décortiquage de fichiers de logs et de rebouchage des failles

Devant la multiplication des sites et des applis web, ce genre d'attaque est en train de devenir de plus en plus répandue, voir peut-être même le risque n°1 pour les entreprises et les administrations !

[kankrelune]

Moi je suis plus mitigé... .. .

Tout dépend de la configuration du serveur (register_global on/off) et de ta façon de coder... .. .

Par contre je vois aux moins deux erreurs dans ta fonction...

1 si $var n'est pas un tableau tu la passe dans sanitize... bien... mais si c'est un tableau ? d'ailleurs si c'est un tableau tu doit avoir une notice car $clean_var n'est pas encore déclarée... .. .

2

Code :

1
2
3
4
5
6
7
8
9
10
 
       if(isset($_POST[$key])){
            $_POST[$key]=$clean_var;
            echo "Post";
        }
 
        if(isset($_GET[$key])){
            $_GET[$key]=$clean_var;
            echo "get";
        }

tu oublis les cookies ($_REQUEST étant un tableau associatif regroupant $_GET, $_POST et $_COOKIE)

Pour le reste il faudrait regarder la fonction sanitize... utiliser une regexp me parait un peu lourd comme traitement après faut voir comment c'est fait... et puis la sécurité à un "cout"... .. .

@ tchaOo°

[illimus]

Moi je prefere etre parano ;-)

Voila par exemple le nettoyage pour les chaines sql

Code :

1
2
3
4
5
6
7
8
9
10
function sanitize_sql_string($string, $min='', $max='')
{
  $string = nice_addslashes($string); //gz
  $pattern = "/;/"; // jp
  $replacement = "";
  $len = strlen($string);
  if((($min != '') && ($len < $min)) || (($max != '') && ($len > $max)))
    return FALSE;
  return preg_replace($pattern, $replacement, $string);
}

[illimus]

Une petite question rapide, tu dis que les variables de session ne sont pas un danger, mais si tu utilises une de tes variables pour une requete ca peut-etre un probleme, donc faut comme meme faire gaff........ attention derriere toi !!! ;-)

[Hervé Saladin]

Je n'ai pas bien compris ta question.

Envoyé par illimus

si tu utilises une de tes variables pour une requete

tu parles d'une variable de session pour une requete sql ?

Envoyé par illimus

ca peut-etre un probleme

Pourquoi ?
Si c'est toi (enfin, ton script plutôt) qui a placé ces variables dans la session (sans que leurs valeur ne soient des parametres d'une requete http ou d'un cookie, bien sûr), il n'existe aucun moyen à l'utilisateur de les modifier, donc je ne vois pas ou est le problème .... ?

Envoyé par illimus

attention derriere toi !!!

AAAAAH ! ou ça ????
T'es con, tu m'a fait peur !

[illimus]

ok c'est ma parano qui me reprend, je croyais que c'etait possible de modifier une variable de session...

[kankrelune]

Code :

1
2
3
4
5
6
7
8
9
10
function sanitize_sql_string($string, $min='', $max='')
{
  $string = nice_addslashes($string); //gz
  $pattern = "/;/"; // jp
  $replacement = "";
  $len = strlen($string);
  if((($min != '') && ($len < $min)) || (($max != '') && ($len > $max)))
    return FALSE;
  return preg_replace($pattern, $replacement, $string);
}

Tu fais un preg_replace pour enlever des ;

Code :

1
2
3
4
5
6
7
8
9
10
11
 
function sanitize_sql_string($string, $min='', $max='')
{
    $string = nice_addslashes($string); //gz
    $len = strlen($string);
 
    if((($min != '') && ($len < $min)) || (($max != '') && ($len > $max)))
        return false;
 
    return str_replace(';', '', $string);
}

Après ta fonction est bien mais pas dans tout les cas... genre sur un int pour une requete sql ok pour éviter une injection mais sur une string ça n'a aucun interet... mysql_real_escape_string() fera très bien le boulot et ça évitera le gars qui poste (par ex) de voir tous les point virgule être supprimés de son texte... .. .

@ tchaOo°

[illimus]

Effectivement mysql_escape_string() a l'air très bien, j'avais pas l'habitude de l'utiliser .... c'est chose faite ;-)

Merci

[kankrelune]

J'allais oublier... pour le nettoyage de l'id utilise (int)$monId ou intval($monId)... si $monId est une string elle sera transformée en int(0)... ça évite des fonctions compliquées pour rien... .. .

@ tchaOo°

[illimus]

en fait pour les id, je crois que c'est bête de faire un "case" particulier, je vais le faire passer dans int.
Par contre je prefere toujours faire une petite expression du style

Code :

$integer = preg_replace("/[^0-9]/", "", $integer);

Car sur un champs code postal (par ex), ca evite de tout basarder si la personne fait un faute de frappe du genre 75e010.

[kankrelune]

Envoyé par illimus

en fait pour les id, je crois que c'est bête de faire un "case" particulier, je vais le faire passer dans int.

Oui... tu peux mettre plusieur case ensemble...

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
switch($type_var[0])
{
    // entier ou id
    case "int":
    case "id":
           $clean_var = sanitize($var, INT);
    break;
 
    // chaine de caratère 
    case "str":
           $clean_var = sanitize($var, HTML);
    break;        
 
    // nombre à virgule
    case "float":
            $clean_var = sanitize($var, FLOAT);
    break;
 
    // si non classé = mode paranoïa ;-)
    default:
         $clean_var = sanitize($var, PARANOID);
}

pour optimiser un peut le tout tu peux virer le switch et mettre

Code :

1
2
3
4
5
6
7
8
9
 
if ($type_var[0] == 'int' || $type_var[0] == 'id')
    $clean_var = sanitize($var, INT);
elseif ($type_var[0] == 'str')
    $clean_var = sanitize($var, HTML);
elseif ($type_var[0] == 'float')
    $clean_var = sanitize($var, INT);
else
    $clean_var = sanitize($var, PARANOID);

Envoyé par illimus

Par contre je prefere toujours faire une petite expression du style

Code :

$integer = preg_replace("/[^0-9]/", "", $integer);

Car sur un champs code postal (par ex), ca evite de tout basarder si la personne fait un faute de frappe du genre 75e010.

Mais bon faire une expression régulière pour tous les int je trouve ça un peu lourd... mais en même temps je comprend ce que tu veux dire... mais tu peux la faire que pour des champs spécifique comme ton code postal et utiliser intval pour les autre ou l'utilisateur n'a pas à se planter... .. .

Moi dans le cas d'une soumission par l'utilisateur je ferais

Une fonction javascript du genre

Code :

1
2
3
4
5
6
 
function testCP(cp)
{
    if(cp.replace(/[^0-9]/g,"") != cp)
        alert("Le code postal entré n'est pas valide");
}

histoire de ne pas soumettre le formulaire pour rien et une vérif en php...

Code :

1
2
3
4
5
 
if(intval($codePostal) != $codePostal)
{
    // on revoit le formulaire avec le message d'erreur
}

Parce que imaginons une vrai faute de frappe genre un + au lieu d'un 9 (touches cote à cote) avec ta fonction tu aura quand même un code postal invalide si par exemple c'est pour l'envoi d'un colis ça peut être génant... .. .

@ tchaOo°

[illimus]

Salut,

Ah je pensais que le switch case était plus rapide que le elseif ?
Avec le elseif tu testes toutes les solutions, c'est plus long ? non ?

Coté javascript à part les controles simples sur les champs obligatoires, j'evite ;-) et encore. Comme c'est des mesures de sécurité, l'utilisateur lamba désactive rarement le javascrit, en revanche le mec qui veux te foutre le bordel sur ton site..... donc pour moi le controle coté server est indispensable.

Au passage comment fait on une attaque xss, c'est pour tester mon site bien sur !!!.