que pensez vous de cette extrait de code ? - PHP - Programmation
Marsh Posté le 22-05-2014 à 12:42:33
Pff, encore un code de stagiaire débutant !
- Bien sûr qu'il faut mettre un espace avant et après le signe égal. C'est une règle élémentaire de typographie. Mais le débutant, lui, il s'en moque. Il écrit n'importe quoi, mettant des espaces sur une ligne et n'en mettant pas sur deux autres, rendant la présentation du code incohérente, donc peu lisible, et donc difficilement maintenable.
Après, tout suit dans la même veine.
- Le code n'a pas de ligne de commentaires.
- Le chemin est écrit en absolu au lieu d'être relatif. Cela rend difficile l'implémentation du code dans différents environnements (de test, de recette, de secours, etc.)
- La suppression se fait sans avertissement, sans trace de ce qui est supprimé.
- Il n'y a pas de contrôle du paramètre $tab_form['code_visuel']. Il pourrait contenir des jokers et d'autres caractères entraînant des suppressions non désirables.
Marsh Posté le 22-05-2014 à 14:21:59
Utiliser exec est toujours un peu risqué d'autant que je doute que ça fonctionne sur tous les OS où PHP est disponible
J'aurais plutôt utilisé la fonction php unlink(), avec, avant, la récupération des fichiers qu'ont veut supprimer. Le code sera peut-être 0.001s plus long à s'exécuter, mais ça sera plus portable.
Et même remarque sur le contenu de $tab_form['code_visuel']. Je voudrais bien savoir comment il prend sa valeur. Parce que si c'est pas bien fait, ça introduit une faille de sécurité
Pour $path_sauvgarde, ce qui me gêne, c'est pas que ça soit un chemin absolu, mais que :
1) son contenu ne provienne pas d'une variable de conf
2) que son contenu ne soit pas généré dynamiquement (ie construit à partir de l'emplacement de stockage du script php).
Pour le commentaire, le bout de code étant très petit, le contexte était peut-être indiqué un peu plus haut. Qq'un qui connait la commande rm -rf comprend ce qui va se passer...
Marsh Posté le 22-05-2014 à 18:48:13
rufo a écrit : Qq'un qui connait la commande rm -rf comprend ce qui va se passer... |
Ouai voila quoi, meme sans connaitre PHP c'est impensable de laisser un truc pareil. Tu soumets un / et boum c'est fini, aucune barriere.
Edit: j'avais pas lu le reste du code, donc je rectifie: tu soumets un ../../../../../ (et p'etre un flag pour le faire sans confirmation) et boum c'est fini.
Marsh Posté le 22-05-2014 à 18:58:35
Si c'était du perl, je mettrais $tab_form['code_visuel'] dans une variable $imfile et je vérifierais que ce n'est pas une chaine vide et que c'est un nom sans risque (pas de sous chaine "../" dedans par exemple), et j'écrirais plutôt $commande_supp = 'rm -rf ' . $path_sauvgarde . '"' . $imfile . '"'; au cas ou le nom de fichier $imfile contiendrait des espaces.
A+,
Marsh Posté le 23-05-2014 à 09:52:18
En PHP, tu peux tout à fait faire les mêmes vérifs
Marsh Posté le 22-05-2014 à 11:15:42
Bonjour,
Je suis en train d'auditer le code d'un de mes clients, et je voudrais votre sur celui là :
Merci :-)
---------------
Du tofu en Alsace : www.tofuhong.com