[PHP/MYSQL] Sécurité suffisante ?

Sécurité suffisante ? [PHP/MYSQL] - PHP - Programmation

Marsh Posté le 08-04-2011 à 16:26:36    

Salut a tous !
 
Je suis en train de sécuriser mon site en ajoutant différents contrôles au niveau des valeurs qui sont utilisées, soit pour afficher des pages, soit pour effectuer des opérations sur ma base, et ainsi prévenir les risques d'injection Mysql, ou autre détournement.
 
Pourriez vous me dire si les contrôles que j'utilise sont suffisant ?
 
Sur cette exemple, c'est j'envoie par la méthode GET "id", qui me sert à générer une page.
 
 

Code :
  1. <?php
  2.  //Vérification de l'existance du GET et de sa nature
  3.  if (isset($_GET['id']) AND is_numeric($_GET['id']))
  4.  {
  5.   // Verification des informations renvoyé dans l'url, puis utilisation des infos dans une requete
  6.   if(get_magic_quotes_gpc())
  7.   {
  8.    $id  = StripSlashes($_GET['id']);
  9.   }
  10.   else
  11.   {
  12.    $id  = $_GET['id'];
  13.   }
  14.   $sql=sprintf(" SELECT  *
  15.       FROM  ma_table
  16.       WHERE id= %d",
  17.       mysql_real_escape_string($id));
  18.   $req = mysql_query($sql) or die('Erreur SQL <br/>'.mysql_error());
  19.   if($data = mysql_fetch_array($req))
  20.   {
  21.    $photoVoiture=  $data['photoVoiture'];
  22.    $photoMoteurF=  $data['photoMoteurF'];
  23.    $photoMoteur=  $data['photoMoteur'];
  24.    $nomVoiture=  $data['nomVoiture'];
  25.    $texteVoiture=  $data['texteVoiture'];
  26.    $videoVoiture=  $data['videoVoiture'];
  27.    $moteurVoiture=  $data['moteurVoiture'];
  28.    $cylVoiture=  $data['cylVoiture'];
  29.    $puissVoiture=   $data['puissVoiture'];
  30.    $accVoiture=  $data['accVoiture'];
  31.    $angleVoiture=  $data['angleVoiture'];
  32.   }
  33.   }
  34.                         // si les contrôles sont faux
  35.                         else
  36.    echo "<p>Cette page n'existe pas</p>";
  37.  ?>


 
 
Je voudrai également savoir, si il faut faire d'une généralité l'utilisation de get_magic_quotes_gpc() et de sprintf() pour TOUTES les requêtes du site, ou uniquement sur les requêtes qui sont alimentées par dés valeurs récupérées en GET, POST
 
Merci


Message édité par kontas le 08-04-2011 à 16:27:02
Reply

Marsh Posté le 08-04-2011 à 16:26:36   

Reply

Marsh Posté le 08-04-2011 à 19:31:13    

idéalement, pour sécuriser tes requêtes, tu devrais utiliser PDO et les requêtes préparées,et de la sorte,  tu t'affranchis de tous les stripslashes, mysql_real_escape_string et consorts.

Reply

Marsh Posté le 08-04-2011 à 20:02:05    

Ca me parait suffisant. Je rajouterais néanmoins les remarques suivantes:
-il vaut mieux ne pas afficher les erreurs mySql mais les logguer
-même remarque pour les erreurs PHP
-si l'ID est bidon, autant renvoyer une erreur 404
-si la requête plante pour une raison x ou y, il faudrait idéalement renvoyer une erreur 500


---------------
Aimer les femmes intelligentes est un plaisir de pédéraste. (Charles Baudelaire) - Vous vulgarisez :o (Jean-Kevin Dubois)
Reply

Marsh Posté le 08-04-2011 à 21:16:56    

+1 pour le coup de masquer les erreurs. Une pratique courante est d'avoir le site dev qui affiche les erreurs toussa toussa, et (ou puis) la prod ou rien n'est affiché si ce n'est un joli message bien formulé et bien généraliste.
 
M'enfin. Déjà, avec le code que tu présente la, tu te protège des trucs les plus basiques comme les injections SQL.
 
Enfin, pour te répondre, ce n'est pas la peine d'échapper TOUTES tes requêtes SQL, mais de manière générale, d'échapper TOUTES les valeurs qui viennent de l'extérieur (donc entre autre de GET/POST).


---------------
mes Photos!
Reply

Marsh Posté le 11-04-2011 à 13:28:12    

Pour les champs texte, utiliser aussi strip_tags() ;) Ca évitera des injections de javascript...


---------------
Astres, outil de help-desk GPL : http://sourceforge.net/projects/astres, ICARE, gestion de conf : http://sourceforge.net/projects/icare, Outil Planeta Calandreta : https://framalibre.org/content/planeta-calandreta
Reply

Marsh Posté le 12-04-2011 à 10:31:14    

bricocoman a écrit :

idéalement, pour sécuriser tes requêtes, tu devrais utiliser PDO et les requêtes préparées,et de la sorte,  tu t'affranchis de tous les stripslashes, mysql_real_escape_string et consorts.


 
En faisant quelles recherche j'étais tombé dessus, peut ont dire que c'est la solution "ultime" contre tous types d'attaques ? l'investissement en temps  de revoir l'ensemble d'un site vaut-il vraiment le coup  ?
 
Merci de ta réponse.
 

philippe06 a écrit :

Ca me parait suffisant. Je rajouterais néanmoins les remarques suivantes:
-il vaut mieux ne pas afficher les erreurs mySql mais les logguer
-même remarque pour les erreurs PHP
-si l'ID est bidon, autant renvoyer une erreur 404
-si la requête plante pour une raison x ou y, il faudrait idéalement renvoyer une erreur 500


 
- Effectivement, c'est vrai que mysql_error renvoi/peut renvoyer des informations comme le nom d'un champs, d'une table ou autres. donc le mieux serais de créer une table qui enregistrerais toutes les erreurs mysql qui peuvent survenir sur le site. interessant !
- J'avais en tête de vérifier aussi l'existance de l'id, histoire d'être sur que les visiteurs ou "hacker" ne tombe pas sur une page foireuse. mais je n'ai pas encore trop réfléchi pour faire au plus simple sans avoir trop de redondance dans le code.
- utiliser l'erreur 500, si je comprend bien, permet au visiteur de comprendre qu'il s'agit d'une erreur interne au site, et non une erreur de chargement d'une page. Mais le visiteur lambda ferat-il la différence ?
 
Merci de ta réponse et de tes conseils ;)
 

knoodrake a écrit :

+1 pour le coup de masquer les erreurs. Une pratique courante est d'avoir le site dev qui affiche les erreurs toussa toussa, et (ou puis) la prod ou rien n'est affiché si ce n'est un joli message bien formulé et bien généraliste.
 
M'enfin. Déjà, avec le code que tu présente la, tu te protège des trucs les plus basiques comme les injections SQL.
 
Enfin, pour te répondre, ce n'est pas la peine d'échapper TOUTES tes requêtes SQL, mais de manière générale, d'échapper TOUTES les valeurs qui viennent de l'extérieur (donc entre autre de GET/POST).


 
 :hello:  
 
Promis, je cacherais mes erreurs Mysql :) , et les valeurs qui sont issue d'un formulaire ? mais pas un formulaire ou une saisi est possible, par exemple comme dans ma page ou il est composé de <select>, j'imagine qu'il pourrais être relativement simple de créer un site parallèle renvoyant sur mon url de validation de mon formulaire, avec des select maison bien crapuleux :O, donc je me répondrais oui, mais toi, qu'en pense tu ?
 
Je me demande aussi si c'est bien utile de vérifier si une valeurs est numérique 2 fois, comme je le fait la avec  
 

Citation :

is_numeric($_GET['id'])


et

Citation :

WHERE id= %d",


 
Dans la doc php de la fonction spintf(), il est indiqué que:

Citation :

d : l'argument est traité comme un entier, et présenté comme un nombre décimal signé.


 
 

rufo a écrit :

Pour les champs texte, utiliser aussi strip_tags() ;) Ca évitera des injections de javascript...


Merci pour le conseil !
 
Mais ou utiliser cette fonction ? faudrait-il substituer get_magic_quotes_gpc() par celle ci ou bien StripSlashes() ? ?

Reply

Marsh Posté le 12-04-2011 à 10:35:41    

strip_tags() ne substitue pas à une autre fonction, elle vient en complément.
 
Tu peux très bien faire une truc du genre :

Code :
  1. $Text = addSlashes(strip_tags($_POST['MonChampTexte']));


---------------
Astres, outil de help-desk GPL : http://sourceforge.net/projects/astres, ICARE, gestion de conf : http://sourceforge.net/projects/icare, Outil Planeta Calandreta : https://framalibre.org/content/planeta-calandreta
Reply

Marsh Posté le 27-04-2011 à 14:36:29    

Salut ! Une fois que tu as verifié que l'ID est numérique, ça sert plus à rien de faire d'autres tests derrière.
En revanche prendre en compte tous les cas utilisateurs (entrée vide ? entrée non numérique ? => message d'erreur) me semble judicieux

Reply

Marsh Posté le 28-04-2011 à 21:00:25    

Salut kontas
 
Tu as eu des réponses sur la sécurité technique. Mais n'oublie pas la sécurité applicative. Je ne connais pas ce qu'il y a en amont de ta page, mais peut être que l'utilisateur n'a pas le droit d'accéder à cet "ID" (et c'est tellement facile de modifier l'ID dans l'url...).  
 
Donc pour ma part, si les données de l'ID à afficher sont confidentielles ou réservées à certaines personnes, alors après le test d'existence de l'id, il conviendrait de rajouter un test "est ce que cet utilisateur a les droits sur cet ID".  
 
Mais si l'ID en question est pour tout le monde, alors oublie mon intervention. DOminique
 

Reply

Sujets relatifs:

Leave a Replay

Make sure you enter the(*)required information where indicate.HTML code is not allowed