Forum |  HardWare.fr | News | Articles | PC | S'identifier | S'inscrire | Shop Recherche
2824 connectés 

  FORUM HardWare.fr
  Programmation
  PHP

  que pensez vous de cette extrait de code ?

 


 Mot :   Pseudo :  
 
Bas de page
Auteur Sujet :

que pensez vous de cette extrait de code ?

n°2228803
PierreC
Posté le 22-05-2014 à 11:15:42  profilanswer
 

Bonjour,
 
   Je suis en train d'auditer le code d'un de mes clients, et je voudrais votre sur celui là :
 

Code :
  1. $path_sauvgarde="/var/www/files/mgc/images_mgc/";
  2. $tab_form = $this->getRequest()->getParams();
  3. $commande_supp="rm -rf ".$path_sauvgarde.$tab_form['code_visuel'];
  4. exec($commande_supp);


Merci :-)


---------------
Du tofu en Alsace : www.tofuhong.com
mood
Publicité
Posté le 22-05-2014 à 11:15:42  profilanswer
 

n°2228809
olivthill
Posté le 22-05-2014 à 12:42:33  profilanswer
 

Pff, encore un code de stagiaire débutant ! [:haha petain]  
 
- 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.

n°2228813
rufo
Pas me confondre avec Lycos!
Posté le 22-05-2014 à 14:21:59  profilanswer
 

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. :D
 
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...


---------------
Astres, outil de help-desk GPL : http://sourceforge.net/projects/astres, ICARE, gestion de conf : http://sourceforge.net/projects/icare, Outil Planeta Calandreta : https://framalibre.org/content/planeta-calandreta
n°2228825
lasnoufle
La seule et unique!
Posté le 22-05-2014 à 18:48:13  profilanswer
 

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.


Message édité par lasnoufle le 22-05-2014 à 18:58:08

---------------
C'était vraiment très intéressant.
n°2228826
gilou
Modérateur
Modzilla
Posté le 22-05-2014 à 18:58:35  profilanswer
 

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+,


Message édité par gilou le 22-05-2014 à 19:00:25

---------------
There's more than what can be linked! --    Iyashikei Anime Forever!    --  AngularJS c'est un framework d'engulé!  --
n°2228856
rufo
Pas me confondre avec Lycos!
Posté le 23-05-2014 à 09:52:18  profilanswer
 

En PHP, tu peux tout à fait faire les mêmes vérifs ;)


---------------
Astres, outil de help-desk GPL : http://sourceforge.net/projects/astres, ICARE, gestion de conf : http://sourceforge.net/projects/icare, Outil Planeta Calandreta : https://framalibre.org/content/planeta-calandreta

Aller à :
Ajouter une réponse
  FORUM HardWare.fr
  Programmation
  PHP

  que pensez vous de cette extrait de code ?

 

Sujets relatifs
Vérificateur de code postalQue pensez vous de mon projet de Web APP
Code pas totalement fonctionnel[WEB] QR CODE et identifiant unique
retour en arrière dans le codeInterprétation d'un code javascript
[AIDE] code HTML/PHP formulaire avec envoi mail automatiquecode pour selection et envoi d'une vidéo à démarrer sur la TV
code complexePHP problème de code
Plus de sujets relatifs à : que pensez vous de cette extrait de code ?


Copyright © 1997-2022 Hardware.fr SARL (Signaler un contenu illicite / Données personnelles) / Groupe LDLC / Shop HFR