Sortie du CMS doorGets 5.0

Posté par  . Édité par Benoît Sibaud. Modéré par ZeroHeure. Licence CC By‑SA.
Étiquettes :
6
29
sept.
2013
PHP

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

  • # ça commence mal

    Posté par  . É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  . Évalué à 0.

      Ce commentaire a été supprimé par l’équipe de modération.

      • [^] # Re: ça commence mal

        Posté par  (site Web personnel) . É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  (site Web personnel) . É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  . Évalué à 0. Dernière modification le 30/09/13 à 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  . Évalué à 1.

            Ce commentaire a été supprimé par l’équipe de modération.

            • [^] # Re: ça commence mal

              Posté par  (site Web personnel) . É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  (site Web personnel) . É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  (site Web personnel) . É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  . Évalué à 0.

                Ce commentaire a été supprimé par l’équipe de modération.

                • [^] # Re: ça commence mal

                  Posté par  (site Web personnel) . É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  . Évalué à 1.

                    Ce commentaire a été supprimé par l’équipe de modération.

                    • [^] # Re: ça commence mal

                      Posté par  (site Web personnel) . É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.

                    • [^] # Re: ça commence mal

                      Posté par  . É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  . É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  . É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  . Évalué à 1.

      Ce commentaire a été supprimé par l’équipe de modération.

      • [^] # Re: ça continue

        Posté par  . É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  . É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  . É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  . É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

  • # injuste

    Posté par  . É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  . Évalué à 3.

      Ce commentaire a été supprimé par l’équipe de modération.

      • [^] # Re: injuste

        Posté par  . É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  . É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 à ceux qui les ont postés. Nous n’en sommes pas responsables.