amélioration du code / suggestion

david7 · 09-02-2010 20:04:20

salut a tous

mon code fonctionne parfaitement mais je souhaite savoir si il y a des améliorations a faire... ou bien des suggestions de votre part :
si oui : donnez moi un maximum d'explications SVP car je ne suis pas un expert du tout..

login.php (connexion a un espace membre)

<?php
// on teste si le visiteur a soumis le formulaire de connexion
if(isset($_POST['connexion'])&&$_POST['connexion']=='Connexion'){
if((isset($_POST['login'])&&!empty($_POST['login']))&&(isset($_POST['pass'])&&!empty($_POST['pass']))){

    $base = mysql_connect ('serveur', 'login', 'password');

    mysql_select_db ('nom_base', $base);

    // on teste si une entrée de la base contient ce couple login / pass

    $sql = 'SELECT count(*) FROM membre WHERE login="'.mysql_escape_string($_POST['login']).'" AND pass_md5="'.mysql_escape_string(md5($_POST['pass'])).'"';

    $req = mysql_query($sql) or die('Erreur SQL !<br />'.$sql.'<br />'.mysql_error());

    $data = mysql_fetch_array($req);

    mysql_free_result($req);

    mysql_close();

    // si on obtient une réponse, alors l'utilisateur est un membre

    if ($data[0] == 1) {

      session_start();

      $_SESSION['login'] = $_POST['login'];

      header('Location: membre.php');

      exit();

    }

    // si on ne trouve aucune réponse, le visiteur s'est trompé soit dans son login, soit dans son mot de passe

    elseif ($data[0] == 0) {

      $erreur = 'Compte non reconnu.';

    }

    // sinon, alors la, il y a un gros problème :)

    else {

      $erreur = 'Probème dans la base de données : plusieurs membres ont les mêmes identifiants de connexion.';

    }

  }

  else {

    $erreur = 'Au moins un des champs est vide.';

  }

}

?>

<html>

<head>

<title>Accueil</title>

</head>

<body>

Connexion à l'espace membre :<br />

<form action="index.php" method="post">

Login : <input type="text" name="login" value="<?php if (isset($_POST['login'])) echo htmlentities(trim($_POST['login'])); ?>"><br />

Mot de passe : <input type="password" name="pass" value="<?php if (isset($_POST['pass'])) echo htmlentities(trim($_POST['pass'])); ?>"><br />

<input type="submit" name="connexion" value="Connexion">

</form>

<a href="inscription.php">Vous inscrire</a>

<?php

if (isset($erreur)) echo '<br /><br />',$erreur;

?>

</body>

</html>

Merci

xTG · 09-02-2010 20:32:23

Ligne 4 : le !empty remplace le travail du isset, ce dernier est donc inutile dans la ligne (le empty est programmé à partir du isset)

Ligne 6-7 : tu ne gères pas les erreurs de connexion, si la connexion ne peut se faire le script sera tout de même chargé actuellement

Ligne 11 : tu affiches la requête à tous les passants du coin en cas d'erreur !!!!! Certains pourraient se faire un malin plaisir à faire joujou avec ta page... Ne jamais donner d'informations sur le script aux visiteurs, c'est une règle d'or !

Ligne 18 : si la requête ne renvoie aucun résultat ta variable ne sera pas initialisée à cet index, il manque donc un isset.

Ligne 25 : idem

Maljuna Kris · 10-02-2010 03:01:42

Saluton,
Moi ce sont les lignes 46 et 47 qui me font froid dans le dos.

xTG · 10-02-2010 08:55:09

De nombreux site le font, PhpBB compris dans le lot (bon sauf pour le mot de passe...).
Qu'est ce que cela peut générer comme faille de sécurité Kris ?
Hormis le mot de passe en clair dans le code html généré.

Dernière modification par xTG (10-02-2010 08:55:33)

david7 · 10-02-2010 12:59:23

"De nombreux site le font, PhpBB compris dans le lot (bon sauf pour le mot de passe...)."
font quoi ?^^

"Qu'est ce que cela peut générer comme faille de sécurité Kris ?
Hormis le mot de passe en clair dans le code html généré."
en clair ?! a quel moment ?

kris : "Moi ce sont les lignes 46 et 47 qui me font froid dans le dos." ok mais pourquoi ?

xTG · 10-02-2010 13:13:42

Le fait de réafficher le pseudo dans le input box lors d'une erreur de connexion.

Dans le code on pourra voir la valeur de ton champs input => le mot de passe en CLAIR. Tu prends donc le scénario de la personne qui se connecte mais tape un mauvais pseudo mais un mot de passe correct. Arrivé sur la page avec l'erreur de connexion il se dit qu'il verra plus tard (ou tout autre chose) puis s'en va sans fermer la page. Une autre personne arrive regarde le code généré par le navigateur et possède le mot de passe. Il n'est pas très difficile ensuite à retrouver le pseudo qui doit surement contenir un caractère en trop ou un truc bénin.

david7 · 11-02-2010 12:14:23

mummm mais... de toute façon j'arrive toujours pas a comprendre "la faille" xTG.. puis toi non plus car tu pose la meme question que moi a Kris :
"Qu'est ce que cela peut générer comme faille de sécurité Kris ?
Hormis le mot de passe en clair dans le code html généré."

car meme avec le mot de passe en clair dans le code HTML, c'est le mot de passe que tapera le visiteur dans le formulaire de connexion donc je vois pas de faille... ?!

Maljuna Kris · 11-02-2010 12:58:25

david7 a écrit :

Hormis le mot de passe en clair dans le code html généré... je ne vois pas de faille... ?!

Elle est pourtant béante, celle-là !

david7 · 11-02-2010 13:17:19

c'est le mot de passe que tape le membre donc ? il verra le mot de passe qu'il tape c'est tout

Maljuna Kris · 11-02-2010 13:32:12

Et le web est un réseau sécurisé, c'est bien connu.

xTG · 11-02-2010 13:43:21

Quant tu utilises un input de type password il y a une bonne raison à ce qu'il ne t'affiche pas les caractères tapés mais des étoiles. ^_-

david7 · 11-02-2010 14:20:56

ba oui.... enfin bref je comprend toujours pas Kris !
le visiteur tape le mot de passe dans le champ password du formulaire, ensuite il clic sur Connexion mais a un message d'erreur il clic-droit "Afficher la source" dans la source il y a le mot de passe en clair (donc le mauvais mot de passe ou meme pseudo qui la tape) et alors ??? s'il a pas les bons login/mot de passe il pourra rien faire donc ??? et alors !

xTG · 11-02-2010 15:22:49

La majorité des erreurs sont des erreurs de frappe... Donc il suffit de changer un caractère pour avoir le bon mot de passe.
Pour un hacker c'est bien plus simple que de trouver le mot de passe en cherchant de 0.

david7 · 11-02-2010 19:26:15

comment le hacker obtient la meme source que le visiteur ? le visiteur chez lui bien tranquil !

xTG · 11-02-2010 19:43:09

Si tu savais tout ce qu'on peut retrouver comme spyware et ce qu'ils font tu poserais pas une question aussi bête. ^^
Tu sais il y a même des logiciels qui enregistrent ce que tu tapes sur ton clavier... Il y en a d'autres qui filment ce que tu regardes et j'en passe.

Bref on peut t'assurer que c'est une faille de sécurité. De plus pourquoi le remettre alors que sur le navigateur on ne verra que des étoiles ? Cela ne sert strictement à rien.

Maljuna Kris · 11-02-2010 20:07:44

Laisse tomber xTG, quand il viendra pleurer ici parce que son site aura été victime d'attaques, il sera probablement plus réceptif à nos conseils.

david7 · 11-02-2010 20:20:24

xTG : oui pas faux pour les étoiles ! je supprime je boss aussi sur t'es premiers conseils dans ton premier post ! merci.

"Laisse tomber xTG, quand il viendra pleurer ici parce que son site aura été victime d'attaques, il sera probablement plus réceptif à nos conseils."
pourquoi tu parle comme cela ??? c'est un forum d'entraide, tu post une seule ligne par post aprés tu veux qu'un debutant (comme moi) arrive a comprendre.. dsl si je comprend pas tout de suite d'un seul coup messieur.

Dernière modification par david7 (11-02-2010 20:53:29)

david7 · 12-02-2010 00:34:21

xTG : pour le empty dans ta premiere remarque c'est comme ceci ?

<?php

// on teste si le visiteur a soumis le formulaire de connexion

if (isset($_POST['connexion']) && $_POST['connexion'] == 'Connexion') {

  if !empty($_POST['login']) && !empty($_POST['pass']) {

Maljuna Kris · 12-02-2010 08:41:09

david7 a écrit :

tu post une seule ligne par post aprés tu veux qu'un debutant (comme moi) arrive a comprendre.. dsl si je comprend pas tout de suite d'un seul coup messieur.

Quand on alerte sur une telle évidence il ne sert à rien de faire une dissertation.
On ne retourne en aucune circonstance un mot de passe en clair un point c'est tout.
Si tu n'es pas capable d'intuiter ça tout seul, ce n'est pas la peine d'espérer aller très loin dans ce domaine.

Quant à la qualité de mes interventions sur ces forums, fais tes preuves et lorsque tu auras rendu le dixième des services que j'ai eu la joie d'apporter ici tu viendras parler, petit bonhomme.

xTG · 12-02-2010 09:13:32

Ouais ouais ouais je suis second !!! Ah hum...

Kris répond à tellement de messages que certains ne sont pas très étoffé mais il faut le comprendre. On ne peut pas prendre le temps de mettre 10 minutes dans la rédaction de chaque message sinon on ne s'en sortirai plus.

Concernant le empty c'est tout à fait cela. Pour la ligne 3 tu peux même optimiser.
Car c'est évalué de gauche à droite. Lorsqu'il verra le ET et qu'il aura évalué la première expression comme fausse il s'arrêtera sans évaluer la seconde.
Donc tu peux remplacer le isset par !empty. Cela revient au même puisque si la variable est vide elle ne peut valoir 'Connexion'.

david7 · 12-02-2010 13:08:09

ah ok donc comme ceci :

<?php

// on teste si le visiteur a soumis le formulaire de connexion

if (!empty($_POST['connexion']) && $_POST['connexion'] == 'Connexion') {

  if !empty($_POST['login']) && !empty($_POST['pass']) {

ou bien :

<?php

// on teste si le visiteur a soumis le formulaire de connexion

if !empty($_POST['connexion'] == 'Connexion') {

  if !empty($_POST['login']) && !empty($_POST['pass']) {

xTG · 13-02-2010 15:14:59

Le premier assurément !
La ligne de ton second code s'ensuit ainsi :

Cas où $_POST['connexion'] = 'Connexion'
if( !empty( $_POST['connexion'] == 'Connexion'] )
if( !empty( true ) )
if( true )
S'éxécute

Cas autre
if( !empty( $_POST['connexion'] == 'Connexion'] )
if( !empty( false) )
if( false)
ne s'éxécute pas (mais pas propre de tester un !empty sur un false...)

Il ne faut pas oublier que chaque expression se remplace par un true ou un false.

Dernière modification par xTG (13-02-2010 15:15:36)

david7 · 13-02-2010 22:02:39

merci pour ta réponse.
sinon j'ai pas conpris : "mais pas propre de tester un !empty sur un false..." c'est a dire ? tu ma dit que je pouvais faire pareil.

xTG · 15-02-2010 12:14:49

On traite un empty sur une variable et non sur un résultat d'expression.
Quand je parlais de faire pareil je parlais bien évidemment de ton code n°1.

david7 · 15-02-2010 14:34:43

ok merci j'ai modifié les 2 premiers points aussi celui de Kris sur la faille.. voici le nouveau code :

* Ligne 6-7 : tu ne gères pas les erreurs de connexion, si la connexion ne peut se faire le script sera tout de même chargé actuellement
jette un oeil pour me dire si c'est bon stp ou si il y a mieu a faire

sinon je comprend pas les points suivants :

* Ligne 11 : tu affiches la requête à tous les passants du coin en cas d'erreur !!!!! Certains pourraient se faire un malin plaisir à faire joujou avec ta page... Ne jamais donner d'informations sur le script aux visiteurs, c'est une règle d'or !
je retire quoi dans la ligne : $req = mysql_query($sql) or die('Erreur SQL !<br />'.$sql.'<br />'.mysql_error());
le $sql sans doute ? mais je pige pas car il y a des ' au debut/a la fin j'efface d'ou a ou ?

* Ligne 18 : si la requête ne renvoie aucun résultat ta variable ne sera pas initialisée à cet index, il manque donc un isset.
je comprend pas (dsl)

* Ligne 25 : idem
je comprend pas (dsl)

voici le nouveau code (attention le numéro des lignes ci-dessus car j'ai modifié donc plus les memes...)

<?php
// on teste si le visiteur a soumis le formulaire de connexion
if(!empty($_POST['connexion'])&&$_POST['connexion']=='Connexion'){
if!empty($_POST['login'])&&!empty($_POST['pass']){

    $mabasededonnee="nombase";

    $base = mysql_connect("localhost","root","motdepasse");

    // test la connection

    if ( ! $base )

    die ("connection impossible");

    // Connecte la base

    mysql_select_db($mabasededonnee) or die ("pas de connection");

    // on teste si une entrée de la base contient ce couple login / pass

    $sql = 'SELECT count(*) FROM membre WHERE login="'.mysql_escape_string($_POST['login']).'" AND pass_md5="'.mysql_escape_string(md5($_POST['pass'])).'"';

    $req = mysql_query($sql) or die('Erreur SQL !<br />'.$sql.'<br />'.mysql_error());

    $data = mysql_fetch_array($req);

    mysql_free_result($req);

    mysql_close();

    // si on obtient une réponse, alors l'utilisateur est un membre

    if ($data[0] == 1) {

      session_start();

      $_SESSION['login'] = $_POST['login'];

      header('Location: membre.php');

      exit();

    }

    // si on ne trouve aucune réponse, le visiteur s'est trompé soit dans son login, soit dans son mot de passe

    elseif ($data[0] == 0) {

      $erreur = 'Compte non reconnu.';

    }

    // sinon, alors la, il y a un gros problème :)

    else {

      $erreur = 'Probème dans la base de données : plusieurs membres ont les mêmes identifiants de connexion.';

    }

  }

  else {

    $erreur = 'Au moins un des champs est vide.';

  }

}

?>

<html>

<head>

<title>Accueil</title>

</head>

<body>

Connexion à l'espace membre :<br />

<form action="index.php" method="post">

Login : <input type="text" name="login" value=""><br />

Mot de passe : <input type="password" name="pass" value=""><br />

<input type="submit" name="connexion" value="Connexion">

</form>

<a href="inscription.php">Vous inscrire</a>

<?php

if (isset($erreur)) echo '<br /><br />',$erreur;

?>

</body>

</html>

PHP|Débutant :: Forums

Advertisement

#1 09-02-2010 20:04:20

amélioration du code / suggestion

#2 09-02-2010 20:32:23

Re : amélioration du code / suggestion

#3 10-02-2010 03:01:42

Re : amélioration du code / suggestion

#4 10-02-2010 08:55:09

Re : amélioration du code / suggestion

#5 10-02-2010 12:59:23

Re : amélioration du code / suggestion

#6 10-02-2010 13:13:42

Re : amélioration du code / suggestion

#7 11-02-2010 12:14:23

Re : amélioration du code / suggestion

#8 11-02-2010 12:58:25

Re : amélioration du code / suggestion

#9 11-02-2010 13:17:19

Re : amélioration du code / suggestion

#10 11-02-2010 13:32:12

Re : amélioration du code / suggestion

#11 11-02-2010 13:43:21

Re : amélioration du code / suggestion

#12 11-02-2010 14:20:56

Re : amélioration du code / suggestion

#13 11-02-2010 15:22:49

Re : amélioration du code / suggestion

#14 11-02-2010 19:26:15

Re : amélioration du code / suggestion

#15 11-02-2010 19:43:09

Re : amélioration du code / suggestion

#16 11-02-2010 20:07:44

Re : amélioration du code / suggestion

#17 11-02-2010 20:20:24

Re : amélioration du code / suggestion

#18 12-02-2010 00:34:21

Re : amélioration du code / suggestion

#19 12-02-2010 08:41:09

Re : amélioration du code / suggestion

#20 12-02-2010 09:13:32

Re : amélioration du code / suggestion

#21 12-02-2010 13:08:09

Re : amélioration du code / suggestion

#22 13-02-2010 15:14:59

Re : amélioration du code / suggestion

#23 13-02-2010 22:02:39

Re : amélioration du code / suggestion

#24 15-02-2010 12:14:49

Re : amélioration du code / suggestion

#25 15-02-2010 14:34:43

Re : amélioration du code / suggestion

Pied de page des forums