Discussion :

[sam01]

Bonjour, à tous,

voilà j'ai subi récemment à deux reprises des attaques sur mon site.
Ces attaques ont eu pour conséquences de supprimer des tables de ma bases de données.

voici un exemple de mes requêtes, pouvez-vous me dire si elles sont bien protégées des injections mysql :

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
// Protège la variable avant l'insertion
function quote_smart($value){
// Stripslashes si nécessaire
        if (get_magic_quotes_gpc()){
                $value = stripslashes_deep($value);
        }
// Protection si ce n'est pas un entier
        if (!is_int($value)){
                $value = "'" . mysql_real_escape_string($value) . "'";
        }
return $value;
}
$sqly = sprintf("delete from flatforswap_adherent where id_adh=%s", quote_smart($_GET['id']));
$reqy = mysql_query($sqly) or die('Erreur SQL : <br />'.$sqly);
 
$sql_des = sprintf("delete from flatforswap_destination where id_adh=%s", quote_smart($_GET['id']));
$req_des = mysql_query($sql_des) or die('Erreur SQL : <br />'.$sql_des);
 
$sql_log = sprintf("delete from flatforswap_logement where id_adh=%s", quote_smart($_GET['id']));
$req_log = mysql_query($sql_log) or die('Erreur SQL : <br />'.$sql_log);

Merci d'avance pour votre aide.

[Bovino]

Ces attaques ont eu pour conséquences de supprimer des tables de ma bases de données.

pouvez-vous me dire si elles sont bien protégées des injections mysql

T'as pas l'impression que la réponse est dans la question ?

[bhamp0]

http://www.developpez.net/forums/d14...tement-videes/
http://www.developpez.net/forums/d14...donnees-mysql/
http://www.developpez.net/forums/d14...ille-requetes/
http://www.developpez.net/forums/d14...vidage-tables/

Va ptet falloir se calmer sur les ouvertures de sujets ...

[nels77]

Essaies ce plugin
https://addons.mozilla.org/fr/firefo...sql-inject-me/

[sam01]

Salut nels77, je l'ai bien testé mais je n'ai rien compris au rapport, de plus je me suis renseigné et il paraît que ce n'est pas fiable...

Une remarque, pour écrire mon code, je me suis inspiré de cet note :

http://php.developpez.com/faq/?page=mysql#mysql-escape

Si elle n'est plus d'actualité et donc plus efficace, pourquoi la laisser toujours en ligne..

[Celira]

Parce qu'elle est toujours valable, tant que tu utilises l'API mysql_. Ce n'est pas parce qu'on a inventé le robot-mixeur qu'on ne peut plus faire de soupe avec un moulin à légumes.
Cela étant dit, l'API mysql_ est considérée comme obsolète et il est recommandé de passer à mysqli ou PDO. (Note : obsolète ne veut pas forcément dire que ça ne marche plus, ça veut dire que ça va disparaitre dans une prochaine version)

A vue de nez, tes requêtes ont l'air propre. Es-tu sûr que le problème vient de là ? Pas d'interface PhpMyAdmin accessible facilement ? Tu es sur un hébergement dédié ?

[ABCIWEB]

Salut,

Dans le titre de ton message, tu dis que tes tables ont été vidées et ensuite tu dis "supprimées"...

Si elles ont été simplement vidées c'est peut-être que tu ne fais pas de contrôle sur le $_GET['id']... Et dans ce cas on peut supprimer ce qu'on veut (en envoyant une série de requêtes avec un id différent) bien qu'il n'y ai pas d'injection sql à proprement parler.

Si tu es entrain de faire ton code, autant utiliser PDO. Avec des requêtes préparées tu as une meilleure sécurité contre les injections sql (mais cela ne te dispenserait pas de contrôler les variables permises dans la requête). Et puis en tant que débutant, autant ne pas apprendre du code obsolète dont le manuel précise qu'il est déconseillé pour les nouveaux projets. Tu es simplement tombé sur un topic un peu ancien...

[Celira]

Effectivement, si ton script de suppression s'appelle par quelque chose du genre delete.php?id=1, il suffit de faire un appel en boucle avec un truc qui incrémente le nombre et pouf! ta base est vide

Donc, il s'agit davantage de validation que de sécurité. Par exemple, ya-t-il un système d'authentification, avec des droits différents ?

[Bovino]

Code :

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

$sqly = sprintf("delete from flatforswap_adherent where id_adh=%s", quote_smart($_GET['id']));

Tu fais quoi par exemple si le paramètre reçu est 1 OR 1=1 ?
Ca transformerait ta requête en

Code sql :

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

delete from flatforswap_adherent where id_adh=1 OR 1=1

et tu laisses passer sans vérification quelconque...

[Yellu]

Bonjour,

Une petite ramarque sur ta fonction :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
function quote_smart($value){
// Stripslashes si nécessaire
        if (get_magic_quotes_gpc()){
                $value = stripslashes_deep($value);
        }
// Protection si ce n'est pas un entier
        if (!is_int($value)){
                $value = "'" . mysql_real_escape_string($value) . "'";
        }
return $value;
}

Comme son nom l'indique, get_magic_quotes_gpc indique le paramétrage de quotes automatiques pour les paramètres GET, POST et COOKIE, d’où le GPC à la fin du nom de la fonction.
Ta fonction n'as pas pour "unique" but de traiter les variables Get, Post et Cookie, puisqu'elle prend une variable en paramètre qui peut très bien être une variable autre que GPC.

Ton test de stripslashes n'as donc rien à faire ici, tu devrai te faire une fonction de récupération de variable GPC qui, elle, posséderait ce test et le retirer de ta fonction de protection de paramètre de requêtes BDD.

[Celira]

Code :

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

$sqly = sprintf("delete from flatforswap_adherent where id_adh=%s", quote_smart($_GET['id']));

Tu fais quoi par exemple si le paramètre reçu est 1 OR 1=1 ?
Ca transformerait ta requête en

Code sql :

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

delete from flatforswap_adherent where id_adh=1 OR 1=1

et tu laisses passer sans vérification quelconque...

Euh... ya quand même mysql_real_escape_string dans sa fonction quote_smart. Sauf erreur de ma part, mysql_real_escape_string sert justement à éviter ce genre de problème.
Ou alors j'ai loupé quelque chose ?

[Bovino]

Bah non, je crois pas... Sauf erreur de ma part (ou de mes yeux)

Code :

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

echo mysql_real_escape_string('1 OR 1=1');

retourne bien

1 OR 1=1

sans échappement.

[BakSh0]

En effet mysql_real_escape_string n'empêchera pas l'utilisateur de passer un 1 OR 1=1.

Mais dans ce cas un intval() ou un cast (int) ou un test avec is_numeric() est une solution.

Attention un intval("1 OR 1=1;"); donnera 1, donc faire un contrôle tout de même pour ne pas faire supprimer un id=1 (comme un admin par exemple)

Cela dit faire un delete d'un paramètre GET sans aucun contrôle ça me parait étrange, ce qui ne t'empêche pas d'utiliser PDO et les requêtes préparées comme le dit à juste titre ABCIWEB.

L'info inutile : (int) serait plus rapide que intval

[sabotage]

Sa fonction quotesmart ajoute les apostrophes, donc on obtient bien WHERE id_adh='1 OR 1=1'Donc soit il existe une chaine de caractères avec du code SQL qui valide is_int() ... mouais, soit le problème vient d'ailleurs : non utilisation de la fonction quotemsart() quelque part ou l'attaque n'est pas une injection SQL.

[Celira]

Je parierais plutôt pour une non-injection, mais là on va pas pouvoir deviner

@Bovino :
Après relecture attentive de la doc :

mysql_real_escape_string() appelle la fonction mysql_escape_string() de la bibliothèque MySQL qui ajoute un anti-slash aux caractères suivants : NULL, \x00, \n, \r, \, ', " et \x1a.

Donc effectivement, ça ne protège pas du bon vieux "OR 1=1". Autant pour moi.

[Bovino]

Autant pour moi.

Je ne temps veux pas, histoire de commettre une faute d'orthographe digne de la tienne !

[sabotage]

En fait l'erreur serait d'écrire

Code :

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

'WHERE colonne_int = ' . mysql_real_escape_string($code_malveillant);

On a l'impression d'avoir tout bien fait et pourtant ...

Je comprends mieux pourquoi PHP encourage les requêtes préparées.

[Celira]

Bref, à l'intention du posteur d'origine : laisse tomber mylsq_ et passes à mylsqi et aux requête préparées.

Je ne temps veux pas, histoire de commettre une faute d'orthographe digne de la tienne !

Tu m'en dira temps c'est p'tet pas reconnu par l'académie française, mais c'est quand même du langage courant. Et franchement, c'est mieux que d'écrire "arrosage" pour "spammer"...

[ABCIWEB]

Bref, à l'intention du posteur d'origine : laisse tomber mylsq_ et passes à mylsqi et aux requête préparées.

Sabotage a cité ici le cas typique où la protection de mysql_real_escape_string est inefficace mais la protection mise en place dans le code original ne permettait pas cette syntaxe comme il le faisait aussi remarquer là. La chaine de caractères étant entourée de quote, un 'OR' ne serait pas interprété comme une commande sql...

Donc si ton conseil est bon sur le principe, dans ce cas cela risque fort de ne pas suffire. Il est très peu probable que l'on puisse réussir une injection sql dans le code montré, le problème vient d'ailleurs, par exemple d'un accès non protégé à ce code, ou d'un vol de session ou... mais passer à mysqli ou à pdo ne résout pas ces problèmes

Autre petite remarque, je conseillerais plutôt à un débutant de passer directement à PDO.
Il vaut mieux réserver l'utilisation de mysqli pour mettre à jour un vieux code mysql si besoin, sinon le passage par mysqli n'est pas une étape intéressante. La syntaxe PDO est infiniment plus agréable à écrire (plus courte) et il existe des fonctionnalités très pratiques (comme passer un tableau de paramètre dans le execute()) qui la rend aussi beaucoup plus souple, sans compter une meilleure portabilité.
Au départ je suis passé de mysql à mysqli par facilité/manque de temps. Et puis pdo étant tellement plus agréable, un an plus tard je me suis mis à pdo. Bah du coup, il a fallu que retransforme tous mes codes génériques de mysqli vers pdo. Au final, j'aurais eu plus vite fait de prendre du temps dès le départ pour passer directement à pdo...