Ce code est il vulnérable aux injections SQL?

Styleman

Touriste
Bonjour,

Voilà j'ai un code que j'utilise depuis longtemps, mais je ne l'ai jamais protéger contre les injections SQL, cela dit, j'essaye de me logger "frauduleusement" grace à des tutoriaux, mais je n'y arrive pas, quelqu'un pourrait me renseigner pour savoir si mon code est vraiment vulnérable?

PHP:
include("config.php");

if($_POST[email]=='' || $_POST[password]=='')
    {
    echo 'Vous avez oublié de remplir un champ.';
	echo "<META HTTP-EQUIV=Refresh CONTENT=2;URL=index.php>";
    exit;
    }

   $sql = "select `password`, `key` from membres where email='$_POST[email]'";
   $req = mysql_query($sql) or die('Erreur SQL !<br><br>'.mysql_error());
   
   $data = mysql_fetch_array($req);

if($data['password'] != md5($_POST[password]))
     {
     echo 'Mauvais login ou pass.';
	 echo "<META HTTP-EQUIV=Refresh CONTENT=2;URL=index.php>";
     exit;
     }
elseif($data[key] != 1) {
	echo "Compte inactif, vérifiez vos mails pour l'activer.";
	echo "<META HTTP-EQUIV=Refresh CONTENT=4;URL=index.php>";
	exit;
}
else
     {
     session_start();
     $_SESSION[login] = $_POST[email];
     session_register('password');
}
Puisque le password doit correspondre à un password de la bdd, ça coincera d'office au niveau de
PHP:
if($data['password'] != md5($_POST[password]))
non?

Merci pour votre aide!
 

Jereck

Α & Ω
Staff
pour répondre à ta question, oui, c'est vulnérable.

Essaie de te logger avec l'adresse email suivante :

'; DROP TABLE membres; --
Ça, ça va te démolir ton site.

Tu peux aussi tester un truc du genre :
'; INSERT INTO membres(email,password,key)VALUES('dtc@dtc.com','7e49f5ea79bf000ff075c265a1eade45',1); --
L'utilisateur ne saura pas se connecter au 1er coup, mais ensuite, il aura un compte actif avec le mdp "DTC"

En gros, il ne faut JAMAIS passer en direct une entrée utilisateur ($_POST[xxx], $_GET[xxx], ...) dans une requête SQL

genre, ici, tu devrait faire un truc genre (me souviens pas de la syntaxe exacte) :
PHP:
$mail = replace($_POST[email], "'", "''");
et utiliser $mail dans ta query
 
1er
OP
S

Styleman

Touriste
Le truc c'est que même comme ça j'arrive jamais a par exemple DROP la table, et puis pour la seconde chose il faudrait qu'il sache le nombre de champs ainsi que la position de email et password, mais cest vrai qu'au bout de quelques combinaisons c'est faisable, ce que je ne comprend pas, c'est pourquoi je n'y arrive pas? Chaque fois j'ai une erreur SQL

Par exemple: Erreur SQL !

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DROP TABLE jeu; --'' at line 1

J'ai créé la table jeu exprès pour essayer de la supprimer comme ça :)
 
1er
OP
S

Styleman

Touriste
PS: merci beaucoup pour ta réponse!
 

tqz_

Elite
et ne pas appeler ses tables membres, clients, etc

et aussi en production ne pas afficher les erreurs SQL dans le browser client la c'est mortel :cool:


sinon comme dit plus haut PDO est INCONTOURNABLE en 2013
 

nicoO

Elite
echo "<META HTTP-EQUIV=Refresh CONTENT=4;URL=index.php>";

ca, c'est moche!
 
1er
OP
S

Styleman

Touriste
Ok merci beaucoup, mais je sais toujours pas faire une injection sql sur mon site, mais en partant du principe qu'il y aura surement des gens plus doués que moi qui iront dessus...

Merci à tous!
 

Skarbone

Le méchant Ω
En gros, il ne faut JAMAIS passer en direct une entrée utilisateur ($_POST[xxx], $_GET[xxx], ...) dans une requête SQL

genre, ici, tu devrait faire un truc genre (me souviens pas de la syntaxe exacte) :
PHP:
$mail = replace($_POST[email], "'", "''");
et utiliser $mail dans ta query
real escape string me parait mieux que replace, quand même :mrgreen:
 

Jereck

Α & Ω
Staff
Ça fait 6 ans que j'ai plus fait de PHP hein ...
 

Skarbone

Le méchant Ω
je dis pas ca mechamment :)
 

Jereck

Α & Ω
Staff
Concernant le fait qu' "il faudrait qu'il sache le nombre de champs ainsi que la position de email et password,", j'ai justement recu un rapport de sécurité tout à l'heure, suite justement à une injection SQL.

La technique consiste à ajouter une clause "HAVING 1=1" à la requête.

Comme cette clause ne peut fonctionner qu'avec un "GROUP BY", la requête plante, avec comme message "column `ID` cannot be used because it is not contained in an aggregation function.

Il suffit alors de pousser dans le champ attaqué un "GROUP BY ID HAVING 1=1".

Et la requête va à nouveau planter, avec le nom de la 2e colonne dans le message d'erreur.

Et ainsi de suite jusqu'à ce que ça ne plante plus, ce qui signifie que l'attaquant a déterminé le nom de tous les champs de la table.
 

Totoleheros83

Petit chat
et ne pas appeler ses tables membres, clients, etc

et aussi en production ne pas afficher les erreurs SQL dans le browser client la c'est mortel :cool:


sinon comme dit plus haut PDO est INCONTOURNABLE en 2013

o_O
Pourquoi pas?

question de sécurité ?

Tu les nommerais comment?
 

aKC1a

OldSchool CS Star
Concernant le fait qu' "il faudrait qu'il sache le nombre de champs ainsi que la position de email et password,", j'ai justement recu un rapport de sécurité tout à l'heure, suite justement à une injection SQL.

La technique consiste à ajouter une clause "HAVING 1=1" à la requête.

Comme cette clause ne peut fonctionner qu'avec un "GROUP BY", la requête plante, avec comme message "column `ID` cannot be used because it is not contained in an aggregation function.

Il suffit alors de pousser dans le champ attaqué un "GROUP BY ID HAVING 1=1".

Et la requête va à nouveau planter, avec le nom de la 2e colonne dans le message d'erreur.

Et ainsi de suite jusqu'à ce que ça ne plante plus, ce qui signifie que l'attaquant a déterminé le nom de tous les champs de la table.
Learning by doing :)
et quand il a tout, ca devient le CACA :p
 

tqz_

Elite
o_O
Pourquoi pas?

question de sécurité ?

Tu les nommerais comment?

j'ai souvent tendance à ajouter un préfixe devant maintenant c'est pas la sécurité la plus important mais tout ce qui peut être fait doit être fait ! Après ça ne pose aucun soucis de faire ça car la table n’apparaît que dans tes classes donc tu ne dois jamais la tapée dans le code.


le plus important est de ne surtout pas afficher les erreurs sur le browser + utiliser PDO + buffer PHP et normalement tu es safe =)
 
Haut