Audit code source PHP / Laravel - Besoin d'avis

Bonjour à tous et toutes, et bonne année.

Je viens ici pour avoir des avis SVP (des avis neutres et des avis réalistes).

Le week-end dernier, une entreprise m’a demandé de faire un léger audit. Entreprise, que je vais ci-dessous nommer "Entreprise cliente".

Tout d’abord, je résume :

Cette "Entreprise cliente" (une PME d’une dizaine de salariés) avait d’abord fait développer son logiciel web (PHP / Laravel / MySQL) par une "Agence web A".

Aujourd’hui cette "Entreprise cliente" est très dépendante de ce logiciel.

Puis un jour, cette "Entreprise cliente" a décidé d’arrêter de travailler avec "Agence web A".

Mais avant de trouver un autre prestataire pour continuer le développement et la maintenance de son logiciel, elle a (en 2024) décidée de faire appel à une "Agence B" (agence spécialisée en audits) afin de faire un audit intégrale du son logiciel web (un audit de code source PHP / Laravel).

Entre parenthèses : cette "Agence B" lui avait facturé 300 € HT de l’heure, pour un total d’environ 10 000 € HT…

Et la conclusion de leur audit est que tout était à peu prèt ok. (l’"Agence B" dit ne pas avoir trouvée d’anomalie dans le code source PHP / Laravel…).

Une fois que cette "Agence B" a fini l’audit du code source, cette "Entreprise cliente" me demande à moi aussi de regarder le code source afin que je donne mon avis. (Et cette "Entreprise cliente" m’a aussi demandé de regarder le PDF du rapport d’audit).

Donc le week-end dernier, j’ai pris une journée afin de faire un tour du code source PHP / Laravel.

Et je constate que l’audit de "Agence B", a : soit été fait à l’arrache, soit pas été fait du tout, ou soit a été fait par un développeur junior (mais vraiment très junior).

Voici un résumé de ce que j’ai pu constater comme problèmes majeurs :

(problèmes majeurs, qui n’ont pas été constatés dans l’audit de "Agence B"…)

- Génération excessive de requêtes SQL sur les pages de listing

La grande majorité des pages de listing de ce logiciel génèrent un nombre extrêmement élevé de requêtes SQL. Certaines pages génèrent des milliers de requêtes SQL (parfois + de 15 000 requêtes SQL !). Car elles ont quasiment toutes des "problèmes N+1", et certaines pages ont une absence totale de pagination, etc.

Voici un exemple :

Laravel Debugbar (un outil connu de debug pour Laravel):

http://www.laravel-cms-preprod.daw-dev.fr/medias/app/upload/Audit-exemple/Page-exemple.png

Et CPU saturé :

http://www.laravel-cms-preprod.daw-dev.fr/medias/app/upload/Audit-exemple/CPU-sature.png

Dans la 2ème image, on peut voir que le CPU du serveur est saturé à 100%.

Je précise que c’est un VPS où il y a seulement ce logiciel d’hébergement.

Et je précise aussi que (à ce moment là, lorsque j’ai pris cette impr écran) que j’étais le seul à utiliser le logiciel. Et que j’ai juste afficher une simple page de listing…

En regardant le code source, on peut très facilement voir ce problème dans (quasiment) toutes les pages de listing…

Voici un simple exemple de code source :

/**
 * Problème dangereux de perf. 19 838 req SQL !
 */
public function all(Request $request)
{
    // Table 'dossier_vendeur' (5300 lignes).
    $dossiers = DossierVendeur::orderBy("ines", "DESC")->orderBy("ref_commercialisation", "DESC");

    if(!empty($request->input("name"))) {
        // ICI : problème dangereux de perf (CPU saturé).
        //Car ici ça charge toutes les lignes de la table 'dossier_vendeur' (5300 lignes).
        $temp_dossiers = DossierVendeur::latest()->get();

        $tabDossierNames = [];
        foreach($temp_dossiers as $temp){
            foreach($temp->vendeurs as $vendeur) {
                $tmp_name = afdt_decrypt($vendeur->last_name);
                if(preg_match("/".$request->input("name")."/i", $tmp_name)){
                    if(!in_array($temp->id, $tabDossierNames))
                        $tabDossierNames[] = $temp->id;
                }
            }
            foreach($temp->partners as $vendeur) {
                $tmp_name = afdt_decrypt($vendeur->title)." ".afdt_decrypt($vendeur->last_name);
                if(preg_match("/".$request->input("name")."/i", $tmp_name)){
                    if(!in_array($temp->id, $tabDossierNames))
                        $tabDossierNames[] = $temp->id;
                }
            }
        }
        if(!empty($tabDossierNames)){
            $dossiers = $dossiers->whereIn("id", $tabDossierNames);
        }
    }

    $count = $dossiers->count();
    $nb_pages = $count != 0 ? ceil($count / 25) : 0;

    $dossiers = $dossiers->skip($page * 25)->take(25)->get();

    return view('back.dossier_vendeur.list', [
        'dossiers' => $dossiers,
    ]);
}

(et dans le code source, quasiment toutes les pages ont ce problème, et dans le code source : c’est flagrant…).

- MySQL - Absence totale de clé étrangère dans la base de données

J’ai pu constater qu’il y a zéro clé étrangère. PS : cette BDD (MySQL) a 59 tables.

- MySQL - Absence d’indexs dans la base de données

Les seuls indexs que j’ai pu voir, sont les clés primaires, et quelques (très rares) indexs d’unicité.

- PHP / MySQL - Absence totale des transactions

J’ai pu constater une absence totale des transactions MySQL.

- Des milliers de fichiers PDF accessibles publiquement

J’ai pu constater que des milliers de fichiers PDF confidentiels, sont accessibles publiquement (sans anthentification) via une simple URL : www.domaine-du-client.fr/5658/pdf-agence/viewer

Vos avis SVP :

Maintenant, qu’en pensez-vous ?

Car pour un audit à 10k €, ne pas avoir trouvé tous ces problèmes majeurs…

Personnellement, je pense qui si l’audit aurait réellement été fait (même par un junior), que ces problèmes aurait été vu, et que l’entreprise cliente aurait été informé la gravité de la situation et sur la criticité de ces problèmes majeurs…

Merci d’avance.

Tous les audits ne recherchent pas la même chose… et visiblement tu te focalises sur la performance par rapport à la base de données (je le précise car on peut s’intéresser à la performance intrinsèque du code PHP et c’est encore différent) mais ce n’est pas le cas de la société d’audit

Et la conclusion de leur audit est que tout était à peu prèt ok. (l’"Agence B" dit ne pas avoir trouvée d’anomalie dans le code source PHP / Laravel…).

Par absence d’anomalie, il s’agit peut-être du fait que rien qui ne ferait planter les scripts ? (est-ce que le boulot est fonctionnellement bien fait est une autre affaire mais déjà pas de page blanche ou de « Internal Server Error » etc.) Ou peut-être est-ce juste qu’il n’y a rien qui permette de pirater le site/serveur ou la base de donnée ? (les société d’audit de sécurité ne font que cela et ne se préoccupent en général pas du reste, mais quelques unes font aussi le point précédent avec des outils d’analyse statique —et non en parcourant manuellement le code comme toi.)

Bref, il te faut voir ce qui a été contractualisé comme prestation (i.e. ce que promettait leur audit) puis si les moyens et résultats mentionnés dans le rapport sont en accord avec cet attendu. Sur ce point, ton entreprise cliente a bien vu mais a besoin d’une personne qui a le bagage nécessaire pour comprendre et expliquer ou contredire le rapport :

(Et cette "Entreprise cliente" m’a aussi demandé de regarder le PDF du rapport d’audit).

Indépendamment de cela, il semble que l’entreprise développeuse initiale sache écrire du PHP et utiliser Laravel, mais ne connait/comprend pas les problématiques de bases de données (outre le code qui rapatrie des tables entières pour opérer sur quelques champs applicativement, le schéma de la base de données n’a pas été vraiment pensé ou optimisé —ce point ne me surprend pas trop à l’heure où on cause très peu Merise, mais je ne vais pas lancer ce débat.)

(Je suppose qu’on parle d’entreprises françaises).

Entre parenthèses : cette "Agence B" lui avait facturé 300 € HT de l’heure, pour un total d’environ 10 000 € HT…

Ce point m’étonne : je ne pense pas que les prix aient doublé en quelques années, donc de mon point de vue, c’est à la fois hors de prix et beaucoup trop court. Avec ce que tu donnes comme indication, ça fait une semaine de boulot environ ; ça laisse à peine le temps de récupérer le code, de le passer dans des moulinettes automatiques style SonarQube, et de rédiger le rapport en fonction. Or, selon la configuration desdits outils, ils ne vont pas remonter les problèmes dont tu parles – ils sont plutôt faits pour de l’audit de sécurité ou de la qualité de code « bête et méchante ». En particulier, c’est rare que ce genre d’audit soit fait sur l’exécution du code ; or les problèmes que tu soulèves sont surtout des problèmes d’exécution. De la même manière, tu mentionnes un audit « un audit de code source PHP / Laravel » et beaucoup d’erreurs SQL ; il est possible que le SQL n’ait même pas été pris en compte dans l’audit, si ça n’était pas explicitement demandé.

À voir ce qui était réellement contractualisé dans cet audit, mais j’ai peur qu’il n’y ait pas grand-chose à faire.

à l’heure où on cause très peu Merise

UML, de nos jours. Déjà lors de mes études il y a 15–20 ans Merise n’était plus mentionné que pour raisons historiques. Cela dit je suis d’accord avec le constat : les BDD sont tellement puissante que trop de conceptions absurdes « fonctionnent ».

Merci pour vos avis.

(Je suppose qu’on parle d’entreprises françaises).

Oui.

c’est rare que ce genre d’audit soit fait sur l’exécution du code ; or les problèmes que tu soulèves sont surtout des problèmes d’exécution.

Dans leur rapport PDF (qu’ils ont facturé environ 10 k) ils ont écrit :

"un environnement similaire à celui de production est mis en place sur l’ordinateur de l’expert"

Ça ne veut malheureusement pas dire grand chose sans plus de détails. Il suffit de lancer une VM avec un OS différent et la bonne version de PHP pour avoir un environnement similaire. Ça ne veut pas dire qu’ils ont utilisé une puissance similaire, ni qu’ils ont fait tourner l’application dans les mêmes conditions ou testé les mêmes pages.

Malheureusement les audits, comme toute prestation, peuvent facilement être bâclés si le contrat n’est pas explicite et exhaustif sur les attentes. C’est comme demander "un résultat de qualité" sans préciser de critères objectifs : c’est sujet à interprétation, donc compliqué à faire appliquer/respecter.

Maintenant rien ne t’empêche de rendre ton propre rapport, qui sera alors complémentaire aux autres. Ça aidera justement ton client à avoir plus d’infos sur son appli, et potentiellement à se poser des questions sur les causes de ces rendus si différents. À ce moment-là tu pourras alors leur indiquer que pour avoir un résultat à la hauteur de leurs attentes il faut déterminer à l’avance les critères (objectifs et mesurables) d’évaluation.

Bonjour,

Pour ajouter à tout ce qui a déjà été dit: ils ont peut-être testé l’application dans un environnement similaire à la production, mais s’ils n’ont pas regardé le code plus que ça, et si la base de donnée était vide au démarrage, ils n’ont pas pu se rendre compte des problèmes de performance. Là encore si rien n’est explicitement spécifié, ils n’avaient probablement pas de données, ou uniquement le minimum pour faire tourner l’application sans erreur.