Ce code est il vulnérable aux injections SQL?

Discussion dans 'Web, design' créé par Styleman, 6 Mars 2013.

  1. Offline
    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!
    Styleman, 6 Mars 2013
    #1
  2. Offline
    Jereck Procrastinateur
    Equipe GamerZ.be
    pour répondre à ta question, oui, c'est vulnérable.

    Essaie de te logger avec l'adresse email suivante :

    Ça, ça va te démolir ton site.

    Tu peux aussi tester un truc du genre :
    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
    Jereck, 6 Mars 2013
    #2
  3. Offline
    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 :)
    Styleman, 6 Mars 2013
    #3
  4. Offline
    Styleman Touriste
    PS: merci beaucoup pour ta réponse!
    Styleman, 6 Mars 2013
    #4
  5. Offline
    Totoleheros83 Petit chat
    Totoleheros83, 6 Mars 2013
    #5
  6. Offline
    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
    tqz_, 7 Mars 2013
    #6
  7. Offline
    nicoO Elite
    echo "<META HTTP-EQUIV=Refresh CONTENT=4;URL=index.php>";

    ca, c'est moche!
    nicoO, 7 Mars 2013
    #7
  8. Offline
    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!
    Styleman, 7 Mars 2013
    #8
  9. Offline
    Skarbone I would rather be snowboarding
    real escape string me parait mieux que replace, quand même :D
    Skarbone, 7 Mars 2013
    #9
  10. Offline
    Jereck Procrastinateur
    Equipe GamerZ.be
    Ça fait 6 ans que j'ai plus fait de PHP hein ...
    Jereck, 7 Mars 2013
    #10
  11. Offline
    Skarbone I would rather be snowboarding
    je dis pas ca mechamment :)
    Skarbone, 7 Mars 2013
    #11
  12. Offline
    Jereck Procrastinateur
    Equipe GamerZ.be
    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.
    Jereck, 7 Mars 2013
    #12
  13. Offline
    Totoleheros83 Petit chat

    o_O
    Pourquoi pas?

    question de sécurité ?

    Tu les nommerais comment?
    Totoleheros83, 8 Mars 2013
    #13
  14. Offline
    aKC1a OldSchool CS Star
    Learning by doing :)
    et quand il a tout, ca devient le CACA :p
    aKC1a, 8 Mars 2013
    #14
  15. Offline
    tqz_ Elite

    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 =)
    tqz_, 8 Mars 2013
    #15