Sortie du CMS doorGets 5.0

Posté par Anonyme le 29 septembre 2013 à 12:01. Édité par Benoît Sibaud. Modéré par ZeroHeure. Licence CC By‑SA.

Étiquettes :

sept.

2013

NdM.: cette dépêche a été réécrite suite à une demande de suppression de son auteur initial. Elle concernait le système de gestion de contenu (CMS) doorGets (PHP+MySQL), en version 5.0.

Aller plus loin

DLFP : Sortie de doorGets CMS V4.1 - CMS PHP open source français (92 clics)

# ça commence mal

Posté par eleg le 29 septembre 2013 à 20:41. Évalué à 6.

tout premier fichier consulté, d’une quarantaine de ligne, dont cinq de code :

https://github.com/doorgets/doorGets/blob/master/index.php

ligne 37 : define('__DOORGETS__','http://www.doorgets.com/');

or http://php.net/manual/bg/userlandnaming.rules.php

PHP reserves all symbols starting with __ as magical. It is recommended that you do not create symbols starting with __ in PHP unless you want to use documented magical functionality

(l’équipe qui développe) PHP se réserve tous les symboles (du langage PHP) commençant par __ (double souligné) comme “magiques”. Il est recommandé que vous ne créiez pas de tels symboles en PHP sauf si vous voulez utiliser des fonctionnalités “magiques” documentées.

voir p. ex. les constantes “magiques” :

http://php.net/manual/fr/language.constants.predefined.php

dont la valeur dépend du contexte de leur utilisation : p. ex. __LINE__ prendra pour valeur le numéro de la ligne où cette “constante” est appelée, et donc deux appels successifs à __LINE__ dans deux lignes de code source différentes renvoient deux valeurs distinctes.

Comportement bien loin de __DOORGETS__, donc. :-(
- [^] # Commentaire supprimé
  
  Posté par Anonyme le 30 septembre 2013 à 02:16. Évalué à 0.
  
  Ce commentaire a été supprimé par l’équipe de modération.
  - [^] # Re: ça commence mal
    
    Posté par LupusMic (site web personnel, Mastodon) le 30 septembre 2013 à 11:52. Évalué à 1.
    
    Pour répondre à tes suppositions (les liens n'indiques rien de ce que tu dis) la seul chose à ne pas faire c'est d'utiliser les constantes prédéfinies de PHP,
    
    Tu plaisantes ?
    
    PHP reserves all symbols starting with __ as magical.It is recommended that you do not create symbols starting with __ in PHP unless you want to use documented magical functionality.
    
    Mais j'avoue, il s'est arrêté sur un détail que j'ai ignoré. Mais la litanie des todo m'a convaincu que c'est à jeter. Sans compter les quelques trous de sécurité évident (usage de $_SERVER sans discernement, par exemple).
    - [^] # Re: ça commence mal
      
      Posté par Simon (site web personnel) le 30 septembre 2013 à 12:11. Évalué à 3.
      
      Ou des XSS qui semblent évidentes https://github.com/doorgets/doorGets/blob/master/setup/doorgets/core/Formulaire.php#L76
      
      Un audit complet de la sécurité est à effectuer avant d'utiliser ce CMS.
      - [^] # Re: ça commence mal
        
        Posté par fabienwang le 30 septembre 2013 à 13:32. Évalué à 0. Dernière modification le 30 septembre 2013 à 13:33.
        
        @Simon, la fonction input seule ne permet pas de dire qu'il y a une faille XSS ici. Il faudrait regarder l'usage de cette fonction input (ce que je n'ai pas fait).
        
        @doorGets, C'est du boulot ce que tu as fait, bravo. Cependant, après avoir testé la démo pendant 15 bonnes minutes, je trouve que ce n'est vraiment pas intuitif.
        
        PS: pour te protéger de failles XSS, il est préconisé d'utiliser les fonctions de filtres dans PHP: http://fr.php.net/manual/fr/book.filter.php
        
        [^] # Commentaire supprimé
        
        Posté par Anonyme le 30 septembre 2013 à 14:44. Évalué à 2.
        
        Ce commentaire a été supprimé par l’équipe de modération.
      - [^] # Commentaire supprimé
        
        Posté par Anonyme le 30 septembre 2013 à 14:04. Évalué à 1.
        
        Ce commentaire a été supprimé par l’équipe de modération.
        
        [^] # Re: ça commence mal
        
        Posté par Simon (site web personnel) le 30 septembre 2013 à 14:44. Évalué à 1.
        
        En effet, avec cette ligne rien ne passe. C'est tout de même confusant d'altérer $_POST lors d'un code review.
        
        [^] # Re: ça commence mal
        
        Posté par LupusMic (site web personnel, Mastodon) le 30 septembre 2013 à 15:55. Évalué à -1.
        
        C'est surtout une extrêmement mauvaise pratique, qui introduit des bogues incompréhensibles. Du genre, on ne peut pas poster d'exemple de code HTML, alors que c'est tout à fait légitime. Une expression mathématique sera tronquée, etc.
        
        [^] # Re: ça commence mal
        
        Posté par LupusMic (site web personnel, Mastodon) le 30 septembre 2013 à 16:11. Évalué à 2.
        
        Faire des audits de sécurité c'est mon job depuis quelques années !
        
        Tu peux nous dire pour qui tu bosses, histoire d'éviter une bande de bras cassés ? Parce que je suis désolé, des choses dans ce goût là :
        
        $checkbox = '<input type="radio" name="'.$name.'" id="'.$name.'_'.$value.'" value="'.$value.'" '.$checked.' >';
        
        C'est poubelle directe. Au-delà du nom, il n'y a aucune traduction de changement de domaine, d'échappement. De plus, on a un emploi abusif, caractéristique des débutants, de l'opérateur de concaténation. Personnellement, j'utilises sprintf pour rendre ça lisible, ou alors je confie cette tâche à un constructeur de requête, qui a la tâche de vérifier que la syntaxe est correcte, et que les éléments soumis sont valides.
        
        Ou encore https://github.com/doorgets/doorGets/blob/master/setup/doorgets/app/models/databaseModel.php#L86 qui fait une redirection en se basant sur le REQUEST_URI, bravo.
        
        Le catch d'exception sans log : https://github.com/doorgets/doorGets/blob/master/setup/doorgets/app/models/databaseModel.php#L109
        
        Trololol, on expose des secrets ('fin, si la classe existe parce qu'elle n'est pas dans le repo) : https://github.com/doorgets/doorGets/blob/master/setup/doorgets/core/CRUD.php#L57
        
        Tiens, on doit pouvoir trouver de l'SQL injection par là, alors que le mammouth PDO est employé :
        https://github.com/doorgets/doorGets/blob/master/setup/doorgets/core/CRUD.php#L91
        
        Bref, du PHP moyen. Et de la part de quelqu'un qui prétend faire de l'audit de sécurité de code, c'est scandaleux.
        
        [^] # Commentaire supprimé
        
        Posté par Anonyme le 30 septembre 2013 à 16:58. Évalué à 0.
        
        Ce commentaire a été supprimé par l’équipe de modération.
        
        [^] # Re: ça commence mal
        
        Posté par LupusMic (site web personnel, Mastodon) le 30 septembre 2013 à 17:40. Évalué à -2.
        
        C'est en court de mise à jour. Mais contrairement à beaucoup, je ne mets pas en ligne des scripts vulnérables.
        
        [^] # Commentaire supprimé
        
        Posté par Anonyme le 30 septembre 2013 à 18:18. Évalué à 1.
        
        Ce commentaire a été supprimé par l’équipe de modération.
        
        [^] # Re: ça commence mal
        
        Posté par LupusMic (site web personnel, Mastodon) le 30 septembre 2013 à 18:44. Évalué à -1.
        
        Mais à ta place, j'aurais honte : publier du code de merde à tout va ; produire des tutoriels inutiles encore plus mauvais que ceux du Site du Zéro ; prétendre que tu audit du PHP alors que tu n'es ni capable d'écrire du code sécurisé, ni en français correct.
        
        J'ai publié du code source, mais je n'en pas fait de publicité, parce que j'estime que ce n'est pas assez bon pour l'être. Par exemple, mon framework qui n'est pas terminé, dont je ne suis pas content, et que j'espère personne n'utiliseras avant que ce ne soit correctement mis en place. Mais j'avoue l'utiliser plus comme laboratoire que comme futur framework qui tue tout.
        
        Du coup, il est évident que Google ne trouve pas mon code. On peut même trouver un petit patch xdebug quelque part. Sans compter que j'ai un nom avec un certain nombre d'homonyme. On peut bien tenter avec mon pseudonyme, mais il faut avouer que depuis les séries médicale, mon pseudonyme est désormais noyé dans l'hystérie médicale autour d'un syndrome rare.
        
        Bref, quand on expose ses productions, il ne faut pas s'étonner qu'on dise que s'est de la merde, surtout quand c'est aussi criant.
        
        [^] # Commentaire supprimé
        
        Posté par Anonyme le 30 septembre 2013 à 19:07. Évalué à 0.
        
        Ce commentaire a été supprimé par l’équipe de modération.
        
        [^] # Re: ça commence mal
        
        Posté par Sekigo le 01 octobre 2013 à 08:12. Évalué à 5.
        
        Que tu ais mal pris les remarques de LupusMic, je peux comprendre. Que tu l’envoies chier, aussi.
        
        Mais que tu répondes en balançant son CV et son ranking google, c'est nul, nul, nul. On dirait de mauvaises pratique par des blogueurs CEO. Et ça n'a aucun putain de rapport avec la choucroute.
        
        Ils t'ont rapportés des bugs de sécurités. Pas de manière cordiales, certes, mais est-ce vraiment si important ? Et ils ont été regardés ton code-source en premier, ce qui est une bonne pratique de la part des utilisateurs-développeurs de ton projet. Moi-même, avant d'installer et d'utiliser une nouvelle bibliothèque python qui n'a pas encore une certaine base utilisateur et confiance (et même dans ce cas là, j'analyse souvent le code source, histoire d'au moins apprendre quelques petits trucs), j'inspecte le code. Et ça me parait normal et sain, comme ça me parait normal de ne pas utiliser la bibliothèque en question si je sens qu'elle est potentiellement dangereuse/compliqué pour rien. Et de rapporter l'erreur au porteur du projet.
        
        On est tous humain, on est pas des machines qui produisent du code secure à la chaine. Ça m'est déjà arrivé pas mal de fois de me prendre des remarques dans la tête parce que mon code comprenait des potentiels trous de sécurité. Dans ce cas, je corrige, et je pousse la personne a cherché d'autres trous, poussant "sournoisement" la personne à devenir contributeur régulier.
# ça continue

Posté par eleg le 30 septembre 2013 à 23:34. Évalué à 2.

Mounir étant ouvert à toute contribution de code…

https://github.com/doorgets/doorGets/blob/master/setup/doorgets/core/Template.php
ligne 42
if(!is_dir($cacheDirectory)){ mkdir($cacheDirectory); }

or http://fr2.php.net/manual/fr/function.mkdir.php

bool mkdir ( string $pathname [, int $mode = 0777 […]

Aïe, le chmod par défaut de création des répertoires par mkdir est 0777 (= ouvert à tous les vents)
- [^] # encore et encore…
  
  Posté par eleg le 01 octobre 2013 à 00:05. Évalué à 0.
  ligne 75 : variable $nameFile créée mais inutilisée (scorie ?)
```
$nameFile = $name.'.php';
```
  (merci à DLFP pour la BD, j’ai bien ri)
- [^] # Commentaire supprimé
  
  Posté par Anonyme le 01 octobre 2013 à 00:09. Évalué à 1.
  
  Ce commentaire a été supprimé par l’équipe de modération.
  - [^] # Re: ça continue
    
    Posté par eleg le 01 octobre 2013 à 01:00. Évalué à 0.
    
    Non, j’en suis plutôt à l’analyse statique du code. ];-]
    
    P. ex. le template header déclare UTF-8, donc tu pourrais avoir dans tes fichiers de locale qqchose comme (cf « vraie » apostrophe, accents, points de suspension… ) :
    
    $_w[371] = "Étape suivante";
    $_w[372] = "Étape précédente";
    $_w[373] = "doorGets est gratuit, offert par Mounir R’Quiba";
    $_w[374] = "Vérification de vos droits d’écriture";
    […]
    $_w[380] = "Votre dossier n’a pas les droits d’écriture…";
    […]
    $_w[394] = "Générer mon site internet doorGets";
    $_w[395] = "Merci";
    $_w[396] = "Vous avez presque fini…"; // euh, oui. ;-)
    
    Je critique, mais il y a aussi des bons côtés : le code est clairement aéré, indenté etc.
  - [^] # Re: ça continue
    
    Posté par eleg le 01 octobre 2013 à 01:08. Évalué à 0.
    
    ce n’est pas la « cata », mais les katas, c’est bon (mangez-en) : prendre l’habitude de tjs définir le chmod dans mkdir c’est réduire le risque de l’oublier le jour où ce sera important.
# c’est que le début… d’accord ? d’accord !

Posté par eleg le 01 octobre 2013 à 00:40. Évalué à 2.
https://github.com/doorgets/doorGets/blob/master/setup/doorgets/core/Template.php
lignes 42 à 74 : remplacer par quelque chose comme (à la louche)
```
// PHP a des fonctions pour ça, !NIH !
$dirName = ('.' == dirname($name) ? '' : dirname($name)) ;
// mkdir est récursif depuis 5.0.0 : Mounir indique 5.3.0 mini pour doorGets
// & d’une pierre deux coups : cas des lignes 42 & 63 couverts
if(!is_dir($cacheDirectory . $dirName)) mkdir($cacheDirectory . $dirName, 0755, true);
$fileTemp = $cacheDirectory . $dirName . basename($name)'.tpl.php';
```
Désolé, je ne fais pas du wheeling à la sortie des écoles parce que le wheeling c’est fun. Idem pour le code, je suis plutôt d’accord avec LupusMic@30/09/13 à 17:40 : je préfère pas de code du tout à du code trivialement dangereux.

Que tou⋅te⋅s celles⋅ceux qui ont maintenant Cabrel dans la tête lèvent le doigt ! SpéciAL dédicace.
# pour finir et boucler la boucle

Posté par eleg le 01 octobre 2013 à 01:15. Évalué à 0.
améliorer la portabilité du code :

https://github.com/doorgets/doorGets/blob/master/setup/doorgets/core/Template.php#L59
```
   $dirNewName .= $explodeName[$z] . PHP_EOL ;
```
cf http://php.net/manual/fr/reserved.constants.php#constant.php-eol

Le bon symbole de fin de ligne pour cette plateforme. Disponible depuis PHP 4.3.10 et PHP 5.0.2
- [^] # Re: pour finir et boucler la boucle
  
  Posté par Joff54 le 01 octobre 2013 à 12:23. Évalué à 1. Dernière modification le 01 octobre 2013 à 12:24.
  Dans le cas présent, ne serait-il pas mieux :
```
$dirNewName .= $explodeName[$z] . DIRECTORY_SEPARATOR ;
```
  http://php.net/manual/fr/dir.constants.php
- [^] # Re: pour finir et boucler la boucle
  
  Posté par barmic le 01 octobre 2013 à 13:16. Évalué à 4.
  
  Si t'a l'intention de faire un audit pourquoi tu ne le ferrais pas sur github ?
  
  Tous les contenus que j'écris ici sont sous licence CC0 (j'abandonne autant que possible mes droits d'auteur sur mes écrits)
- [^] # Re: pour finir et boucler la boucle
  
  Posté par barmic le 01 octobre 2013 à 13:35. Évalué à 3.
  Note tout de même que le problème c'est surtout de la ligne 54 à 69 à vu de nez ça devrait être remplacé par :
```
mkdir(join(DIRECTORY_SEPARATOR, $explodeName), $mode, TRUE)
```
  Tous les contenus que j'écris ici sont sous licence CC0 (j'abandonne autant que possible mes droits d'auteur sur mes écrits)
  - [^] # Re: pour finir et boucler la boucle
    
    Posté par LupusMic (site web personnel, Mastodon) le 01 octobre 2013 à 17:26. Évalué à 1.
    
    Et le chunk 110,114 qui devrait être remplacé par un strtr.
    
    https://github.com/doorgets/doorGets/blob/master/setup/doorgets/core/Template.php#L110
    
    Mais le plus gros problème, c'est de laisser un accès en écriture à une application PHP.
# injuste

Posté par did31 le 01 octobre 2013 à 22:58. Évalué à 5.

Effarant ce que j'ai pu lire ici.
Peut-etre que le code de Mounir n'est pas peut-etre pas exceptionnel (je ne suis pas capable de juger et je ne me permettrai pas). Okay. Il y a un certain nombre de vulnérabilité. Soit.
Mais le pourrir a ce point la, excusez moi, ce n'est pas du tout constructif. Si vous vouliez l'etre, ouvrez des PR sur github. C'est gratuit, ca aide a tracer le problème et c'est visible par les gens qui voudraient utiliser son CMS.
Au lieu de ca, vous utilisez LinuxFR pour le discrediter. Le gars a pose ses "corones" sur la table pour publier son projet a la vue de TOUS. C'est louable de sa part. Il a fait des erreurs ?! Non ? Incroyable car les gens bons ne font jamais d'erreurs (connes), c'est bien connu.
C'est facile, hein, de critiquer tout en restant cacher et de ne jamais publier son code, non ? Pfff.

Alors oui, bravo, vous vous etes fait moussés en rabaissant quelqu'un. Bien joué et on a tous vu que vous etiez de vrais cadors du PHP (waouh). Permettez moi de vous dire que je ne voudrais pas de gars comme vous dans mon equipe meme si vous etiez de la core team PHP (vous en etes non ?).

Quel manque de maturité et de savoir-vivre. En esperant qu'a l'avenir, vous fassiez preuve d'un peu plus de modestie et de respect.
- [^] # Commentaire supprimé
  
  Posté par Anonyme le 02 octobre 2013 à 01:40. Évalué à 3.
  
  Ce commentaire a été supprimé par l’équipe de modération.
  - [^] # Re: injuste
    
    Posté par barmic le 02 octobre 2013 à 02:03. Évalué à 4.
    il faut prendre les points objectifs de la masse de critique pour avancer,
    
    Garde cet état d'esprit :)
    
    mais bon ça vaut le sacrifice !
    
    Super. Je doute que tu fasse ça pour la reconnaissance (sinon tu as mal choisi ton domaine…), continue à faire ce qui te plaît, essaie de ne pas trop prendre pour toi les critiques et essaie d'améliorer la communication :
    - Pourquoi tu as fais ce CMS ?
    - Quels sont ses avantages et ces inconvénients ?
    - As-tu des axes d'amélioration pour la prochaine version ?
    Tous les contenus que j'écris ici sont sous licence CC0 (j'abandonne autant que possible mes droits d'auteur sur mes écrits)
- [^] # Re: injuste
  
  Posté par barmic le 02 octobre 2013 à 01:57. Évalué à 4.
  Quel manque de maturité et de savoir-vivre. En esperant qu'a l'avenir, vous fassiez preuve d'un peu plus de modestie et de respect.
  1. de base ça pars mal, la dépêche est assez pauvre. On a droit à des listes à points pour décrire un CMS. Des CMS il en existe des pelles (surtout en PHP), si personne ne prends le temps de mettre en avant l'intérêt (au moins pour l'auteur) il ne faut pas s'attendre à avoir un accueil dithyrambique
  2. la plus part des critiques sont argumentées, elles sont abruptes, elles sont dures et peuvent être mal prise avec raison, mais elles sont argumentées et la plupart donnent même une solution !
  3. doorGets a rapidement fait des attaques ad hominem (ou à se mettre en avant) c'est pas particulièrement malin (alors que répondre technique à des arguments techniques c'est logique)
  4. c'est tout de même dommage que j'ai pu faire une remarque alors que je n'ai jamais fais de PHP dans ma vie
  Par contre j'ai pertiné ton commentaire et presque tous les siens ainsi que cette dépêche car en effet il met les couilles sur la table et qu'il n'y a que ceux qui essaient qui font des erreurs. Il faut pas se décourager ou se braquer, je ne doute pas que son CMS est bien (je ne sais pas trop ce qui le distingue des autres par contre), il y a juste quelques erreurs de jeunesse qui seront rapidement corrigées.
  
  Tous les contenus que j'écris ici sont sous licence CC0 (j'abandonne autant que possible mes droits d'auteur sur mes écrits)

Suivre le flux des commentaires

Note : les commentaires appartiennent à celles et ceux qui les ont postés. Nous n’en sommes pas responsables.