Revue de code sécurité : ce que l'humain trouve, pas
le SAST

Deux relecteurs regardent le même fichier. Le premier est un humain qui connaît le métier de l'application ; le second est un moteur d'analyse statique qui suit la donnée de sa source à son point d'arrivée. Ils ne cochent pas les mêmes cases. Pour le vérifier autrement qu'en théorie, j'ai pris un dépôt volontairement vulnérable, connu et public, et je l'ai audité deux fois : une revue de code à la main, puis un scan semgrep avec les règles communautaires. Voici les deux listes de failles, l'une à côté de l'autre.

Le résultat n'est pas "l'humain gagne" ni "l'outil gagne". Chacun a trouvé des choses que l'autre a laissées passer, et c'est précisément l'intérêt de l'exercice. Une revue de code sécurité sérieuse combine les deux, parce qu'ils sont aveugles à des endroits différents.

Revue de code sécurité : deux méthodes, un seul dépôt

Un audit de code source peut se mener de deux façons. La première est humaine : on ouvre les routes, on suit les entrées, on lit la logique d'autorisation. La seconde est automatisée : un outil SAST (Static Application Security Testing) parcourt le code sans jamais l'exécuter, cherche des motifs dangereux et trace le flux d'une donnée non fiable jusqu'à un point sensible. Les deux répondent à la même question, "où est la faille", mais avec des yeux différents. Pour situer ces familles d'outils les unes par rapport aux autres, l'article sur le SAST, DAST, IAST et l'IA pose le décor ; ici, on reste sur l'analyse statique et la relecture.

NodeGoat est un bon banc d'essai : environ 2 200 lignes de JavaScript côté serveur, assez petit pour être lu entièrement en une session, et volontairement criblé de vulnérabilités qui correspondent aux catégories du Top 10 de l'OWASP. Chaque faille y côtoie son correctif, laissé en commentaire, ce qui rend l'évaluation honnête : on sait ce qui est vulnérable et pourquoi. La commande du scan tient en quelques lignes.

# semgrep 1.176.0, dépôt OWASP/NodeGoat @ c5cb68a
semgrep scan \
  --config p/javascript --config p/nodejs --config p/expressjs \
  --config p/security-audit --config p/owasp-top-ten \
  app/

# Ran 98 rules on 43 files: 10 findings.

Quatre-vingt-dix-huit règles, quarante-trois fichiers, dix résultats. C'est peu, et c'est déjà instructif : sur ces dix, une partie vise du code serveur réellement dangereux, une autre partie tombe sur des fichiers de documentation. Regardons ce qui est remonté.

Ce que le SAST a trouvé seul

Le scanner brille sur les failles qui ont une forme : une entrée de requête qui atterrit sur une fonction dangereuse. Le cas d'école est ici une injection de code par eval, dans le handler des cotisations. Trois lignes, trois findings, chacun classé CWE-95.

// app/routes/contributions.js
this.handleContributionsUpdate = (req, res, next) => {
    // Insecure use of eval() to parse inputs
    const preTax = eval(req.body.preTax);     // <-- semgrep flag CWE-95
    const afterTax = eval(req.body.afterTax);
    const roth = eval(req.body.roth);
    // ...
};

Le message de semgrep est net : "Found data from an Express or Next web request flowing to eval." C'est exactement le genre de raisonnement qu'un moteur d'analyse de flux fait bien et vite : la donnée part de req.body, elle n'est ni validée ni transformée, elle arrive sur eval. Aucune ambiguïté, aucune fatigue, et ça retombe dans le mille à chaque commit. Le scan a aussi épinglé une redirection ouverte (CWE-601) où l'URL de destination est prise telle quelle dans req.query.url, et un cross-site scripting réfléchi dans un gabarit HTML (CWE-79) où une variable est injectée dans un attribut href. Voici la liste brute, telle qu'elle sort.

Cinq de ces dix findings visent des liens http:// dans des pages de tutoriel. La règle a raison sur le principe (transport en clair, CWE-319), mais elle se déclenche sur du contenu pédagogique, pas sur une faille exploitable. C'est le lot de tout scan : une part de bruit qu'il faut trier. Retenez surtout ce que la machine a vu toute seule : l'eval, la redirection, le XSS de gabarit. Ce dernier point mérite d'être noté, car un relecteur humain pressé aurait très bien pu survoler ce partial HTML sans le voir. L'outil, lui, ne saute aucune ligne.

Ce que la revue manuelle a trouvé seul

Maintenant, la lecture à la main. Elle commence toujours par les routes, parce que c'est là que se décide qui a le droit de faire quoi. La page des allocations expose sa faille dès la déclaration de la route : l'identifiant d'utilisateur est pris dans l'URL.

// app/routes/allocations.js
this.displayAllocations = (req, res, next) => {
    // Fix (en commentaire dans le dépôt) : prendre userId dans la SESSION
    // const { userId } = req.session;
    const { userId } = req.params;      // <-- vient de l'URL
    const { threshold } = req.query;

    allocationsDAO.getByUserIdAndThreshold(userId, threshold, (err, allocations) => {
        // ... renvoie les allocations de CE userId, quel qu'il soit
    });
};

La route est GET /allocations/:userId et elle est bien protégée par un middleware isLoggedIn. Sauf qu'être connecté ne dit rien sur le droit de lire ce compte-là. Un utilisateur authentifié qui change /allocations/42 en /allocations/43 lit le portefeuille de son voisin. C'est une référence directe à un objet non sécurisée, un IDOR, et c'est le cas le plus pur de faille que le SAST ne voit pas : le code est parfaitement valide. Il lit un paramètre, interroge la base, renvoie le résultat. Il n'y a aucune fonction dangereuse à signaler. Ce qui cloche, c'est l'intention.

Le même fichier de données cache une seconde faille, plus vicieuse, que le scan a également ratée : une injection NoSQL. Le filtre de recherche construit une clause $where de MongoDB en concaténant directement le paramètre threshold de l'utilisateur.

// app/data/allocations-dao.js
if (threshold) {
    return {
        // threshold vient du client, injecté sans échappement
        $where: `this.userId == ${parsedUserId} && this.stocks > '${threshold}'`
    };
}

L'opérateur $where de MongoDB exécute une expression JavaScript côté serveur pour chaque document. Concaténer une entrée utilisateur dedans, c'est offrir l'exécution de JavaScript arbitraire dans le moteur de base. Un threshold valant 0';while(true){}' transforme la requête en déni de service. Cette faille est cousine des injections SQL classiques, mais le sink est spécifique à MongoDB. Les règles communautaires que j'ai chargées connaissent eval ; elles n'avaient pas de motif pour $where. Un relecteur qui sait ce que fait cet opérateur le repère en une seconde.

Enfin, l'authentification. Le stockage et la vérification des mots de passe se font en clair, la version sécurisée avec bcrypt étant restée en commentaire dans le code du dépôt.

// app/data/user-dao.js  (validateLogin)
const comparePassword = (fromDB, fromUser) => {
    return fromDB === fromUser;              // comparaison en clair
    // Fix : return bcrypt.compareSync(fromDB, fromUser);
};

// app/data/user-dao.js  (addUser)
const user = {
    userName, firstName, lastName,
    password                                  // stocké tel quel
    // Fix : password: bcrypt.hashSync(password, bcrypt.genSaltSync())
};

Aucun appel de fonction dangereuse ici non plus : un === et une affectation d'objet ne déclenchent aucune règle. Il faut comprendre que password doit être haché pour voir le problème. Même logique pour la route d'administration des avantages sociaux : elle vérifie qu'on est connecté, jamais qu'on est administrateur. Le correctif prévu, ajoutant un middleware isAdmin, dort en commentaire.

// app/routes/index.js
app.get("/benefits", isLoggedIn, benefitsHandler.displayBenefits);
// Fix A7 (Function Level Access Control), resté en commentaire :
// app.get("/benefits", isLoggedIn, isAdmin, benefitsHandler.displayBenefits);

Pourquoi le SAST rate le contrôle d'accès

Le point commun de l'IDOR, du mot de passe en clair et du isAdmin manquant : ce sont des failles sans sink. Un moteur d'analyse statique raisonne par flux de données. Il part d'une source (une entrée non fiable), suit ses transformations, et alerte quand elle atteint un sink (une fonction sensible : eval, une requête SQL, une écriture de fichier). Ce modèle est redoutable pour les injections. Il est structurellement aveugle au contrôle d'accès, parce qu'un contrôle d'accès manquant n'est pas une ligne de code fautive : c'est une ligne absente. On ne peut pas faire correspondre un motif à quelque chose qui n'existe pas.

C'est aussi pour cela que le contrôle d'accès défaillant est en tête du Top 10 de l'OWASP, catégorie A01 : c'est à la fois la faille la plus répandue et l'une des plus difficiles à détecter par un outil, faute de critère automatique. Décider que /allocations/43 est interdit à l'utilisateur 42 suppose de connaître le modèle d'autorisation de l'application, que l'outil ignore. Le secure coding repose donc sur une discipline humaine que le scanner ne remplace pas : traiter tout identifiant venu du client comme suspect, et rattacher chaque objet à la session qui a le droit de le lire. Poser ces règles au bon moment relève du modèle de menaces, en amont du code.

Pourquoi la revue manuelle rate des sinks

L'inverse est vrai aussi, et il faut être honnête sur ce point. Le XSS de gabarit que semgrep a épinglé se trouvait dans un fichier HTML, ligne 78, noyé au milieu de balises. Sur un dépôt de deux mille lignes, un humain concentré le voit ; sur cinquante mille, après trois heures de lecture, il glisse dessus. La machine ne connaît pas la lassitude, ne saute pas de fichier, et applique la même règle à la première comme à la dix-millième ligne. C'est précisément ce que la relecture humaine fait mal : la couverture exhaustive et régulière.

Il y a une nuance à ne pas gommer : le $where que le scan a manqué n'est pas hors de portée d'un SAST par nature. Une règle dédiée à cet opérateur, ou un moteur avec une base de sinks NoSQL plus riche, l'aurait attrapé. Ce que la mesure montre, c'est que les règles génériques prêtes à l'emploi ne couvrent pas tout : la qualité d'un scan dépend du jeu de règles, et personne n'avait écrit celle-là pour ce code. L'IDOR et le contrôle d'accès, eux, restent hors de portée quel que soit le jeu de règles, parce qu'ils n'ont pas de sink du tout. La différence est là.

La lecture de ce partage tient en une phrase : la colonne centrale, ce sont les failles à sink, que les deux méthodes attrapent ; les colonnes latérales, ce sont les angles morts respectifs. Le SAST couvre le volume et ne fatigue pas ; la relecture couvre l'intention et le métier. Retirer l'une des deux, c'est garder un angle mort entier.

Combiner les deux : la revue de code en pratique

Les code review best practices qui tiennent la route ne choisissent pas entre l'outil et l'humain, elles les empilent. Concrètement :

• Le SAST est le plancher, dans la CI. Il tourne à chaque commit, casse la build sur un sink introduit, et sert de mémoire qui n'oublie rien. Le brancher tôt évite de découvrir une injection en fin de sprint ; c'est tout l'objet d'un SAST et DAST intégrés à la CI/CD.
• La relecture humaine se concentre là où l'outil est aveugle. Une fois le plancher automatisé posé, le temps humain, qui est cher, va au contrôle d'accès, à la logique métier et à la gestion des secrets, pas à re-chercher des eval que le scanner voit déjà.
• Le modèle de menaces oriente la lecture. Savoir quelles données sont sensibles et qui a le droit d'y toucher transforme une relecture ligne à ligne en une chasse ciblée. C'est le même réflexe que face au code généré par IA, où le contrôle d'accès est justement ce que le modèle oublie le plus.

Cette division du travail recoupe la distinction entre les approches de test : un scan automatique est un regard whitebox partiel, une revue humaine est un whitebox complet. L'article sur le pentest black box, white box et grey box détaille pourquoi l'accès au code change tout ce qu'on peut trouver. Et le tri des failles remontées, leur priorisation et leur suivi, relèvent ensuite de la gestion des vulnérabilités : trouver n'est que la première moitié du travail.

À retenir

• Sur le même dépôt, semgrep a trouvé les failles à sink (injection eval, redirection ouverte, XSS de gabarit) et une part de bruit ; la revue manuelle a trouvé l'IDOR, le mot de passe en clair, l'injection $where et l'absence de contrôle d'administrateur.
• Un SAST est aveugle au contrôle d'accès par construction : une autorisation manquante est une ligne absente, pas un motif à détecter.
• Une relecture humaine rate des sinks noyés dans le volume par fatigue ; l'outil, lui, ne saute aucune ligne et tourne à chaque commit.
• La qualité d'un scan dépend du jeu de règles : le $where manqué l'était par manque de règle dédiée, pas par impossibilité de principe. Le contrôle d'accès, lui, reste hors de portée.
• En pratique : le SAST en plancher automatisé dans la CI, la relecture humaine sur le contrôle d'accès et la logique métier, le tout orienté par un modèle de menaces.

Si vous voulez une lecture humaine de votre code, celle qui attrape les IDOR et les règles métier contournables qu'aucun scanner ne remonte, c'est le métier d'une revue de code whitebox. Et pour le plancher automatisé qui tourne à chaque commit, le module SAST de SecAI s'en charge en continu. Pour en parler avec un humain, la page contact est directe.

Questions fréquentes sur la revue de code sécurité