fix(security): ferme les 6 alertes CodeQL + 3 alertes Dependabot - #45
Open
Kvnbbg wants to merge 4 commits into
Open
fix(security): ferme les 6 alertes CodeQL + 3 alertes Dependabot#45Kvnbbg wants to merge 4 commits into
Kvnbbg wants to merge 4 commits into
Conversation
server/index.js (#6, #7 — critical, type confusion) `req.files ?? []` ne garde que null/undefined, pas le type. Si req.files est un objet (ce que multer produit avec .fields()/.single()), files.length vaut undefined et `undefined > MAX` est false : les deux limites d'upload cessent silencieusement de s'appliquer. Remplacé par Array.isArray(). terminal-plugins/{moltbook,french-dev-social}.mjs (#4, #8 — bad-tag-filter) /<script[\s\S]*?<\/script>/ ne matche pas `</script >`, donc le corps du script fuitait dans le texte extrait. Ce texte alimente le prompt de Laura : le risque réel ici est l'injection de prompt, pas le XSS. Ajout de \b et \s* sur les balises fermantes, et suppression des commentaires HTML. Vérifié : `</script >`, `</script\n>`, `</style >` ne fuient plus. src/components/ChatWidget.tsx (#5 — xss-through-dom) href recevait la saisie utilisateur validée par un simple préfixe regex. Remplacé par un parsing `new URL` + liste blanche de protocoles ; tout ce qui échoue retombe en texte brut. Durcissement (le préfixe bloquait déjà javascript:), pas une faille exploitée. .github/workflows/ci.yml (#1 — missing workflow permissions) Ajout de `permissions: contents: read`. Vérifié : npm run lint (0), tsc --noEmit (0), vitest 29/29. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFfrQjSxUQ5EPDFNt6Qd6r
Lockfile uniquement — la plage déclarée (^7.14.1) était déjà satisfaisante, les versions vulnérables étaient juste figées dans le lock. - react-router / react-router-dom 7.18.1 -> 7.18.3 (HIGH, GHSA-qwww-vcr4-c8h2, contournement CSRF en mode RSC) - js-yaml -> 4.3.2 (HIGH, consommation CPU quadratique sur !!omap) - postcss -> 8.5.26 (MEDIUM, correctif incomplet de GHSA-6g55-p6wh-862q) npm audit : 0 vulnérabilité. Vérifié : vitest 29/29, build OK. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFfrQjSxUQ5EPDFNt6Qd6r
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Le correctif CodeQL précédent déplaçait le retrait des commentaires avant celui des <script>/<style>. Un « <!-- » isolé dans un corps de script (légal en JS via les commentaires HTML-like de l'Annexe B, fréquent dans les bundles minifiés) faisait alors courir la correspondance paresseuse au-delà de </script> jusqu'au « --> » suivant : le terminateur était consommé, la regex script ne matchait plus, et le code JS brut se retrouvait dans le texte — puis dans le prompt LLM via callBridge, depuis une page distante contrôlable par un tiers. L'ordre inverse a le défaut miroir : un <script> commenté avalait le texte visible qui suivait. Une alternation unique, balayée de gauche à droite, n'a aucun des deux défauts : la construction qui s'ouvre en premier gagne. Ajoute les tests de non-régression correspondants ; terminal-plugins/ n'avait aucune couverture, d'où la régression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012k6VRd5iRauyF9xndGLMe9
| // either way: comments-first lets a stray `<!--` inside a script body run | ||
| // past `</script>` and leak raw JS into the prompt, while scripts-first | ||
| // lets a commented-out `<script>` swallow the visible text after it. | ||
| .replace(/<!--[\s\S]*?-->|<script\b[\s\S]*?<\/script\s*>|<style\b[\s\S]*?<\/style\s*>/gi, ' ') |
| // past `</script>` and leak raw JS into the prompt below, while | ||
| // scripts-first lets a commented-out `<script>` swallow the visible text | ||
| // after it. A single pass has neither failure. | ||
| .replace(/<!--[\s\S]*?-->|<script\b[\s\S]*?<\/script\s*>|<style\b[\s\S]*?<\/style\s*>/gi, ' ') |
DÉPENDANCES — qs 6.15.3 -> 6.16.0 : contournement de la limite de tableau par virgule dans une clé entre crochets (GHSA-x5fp-wj9c-mxmx) et déni de service par isBuffer contrôlé par l'attaquant (GHSA-4mjr-xmp4-gh2g). `npm audit fix` ne pouvait rien : qs vient d'express@4.22.2, déjà la dernière 4.x, et la seule voie automatique passait par Express 5 — une rupture majeure pour un correctif mineur. Un `overrides` cible la dépendance transitive sans toucher à Express, et le bloc existait déjà dans ce package.json (cookie, send, serve-static, body-parser) : c'est la convention du projet, pas une de plus. SCAN — la règle `public-local-or-private-origin` échouait à CHAQUE exécution, sur `http://localhost` NU. Cette chaîne est la base de repli de react-router lorsque `location.origin` vaut « null » (contexte bac à sable), inlinée dans tout bundle. Elle ne joint rien et ne révèle rien. Un contrôle rouge en permanence cesse d'être lu, et c'est alors la vraie fuite qui passe : le bruit coûte plus cher que l'absence de règle. Vérifié sur la branche de sauvegarde — le scan échouait déjà avant ce lot, ce n'est pas une régression du correctif qs. La règle distingue désormais une adresse de DÉVELOPPEMENT (port ou chemin : `localhost:5173/api`, IP privée complète, `/home/…`, `file:///…`) du repli nu. Deux resserrages sont venus des tests eux-mêmes : - `10.0.\d` attrapait la version « 10.0.1 » -> IP complète exigée ; - `localhost[:/]\d` MANQUAIT `http://localhost/api/admin`, un chemin commençant par une lettre -> `[\w-]`. Les fichiers de test sont exclus du scan : les tests d'un détecteur portent par construction des échantillons de ce qu'il détecte, et signaler le garde parce qu'il connaît le visage du voleur n'apprend rien. L'exclusion ne touche aucun code livré. 13 cas figent la frontière dans les deux sens — ce qui doit encore être attrapé, ce qui ne doit plus l'être — et un test compare le motif du test à celui du script, deux copies d'une même règle finissant toujours par diverger. Vérifié : 0 vulnérabilité sur l'audit, build vert, 52 tests verts, scan vert, et détection confirmée en déposant une vraie fuite dans public/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ferme #1, #4, #5, #6, #7, #8 (CodeQL) et les 3 alertes Dependabot ouvertes.
#6 / #7 — critical : une garde qui échoue en mode ouvert
??protège contrenull/undefined, pas contre le type. Sireq.filesest un objet — ce que multer produit dès qu'on passe de.array()à.fields()ou.single()— alorsfiles.lengthvautundefined, etundefined > MAX_FILES_PER_UPLOADestfalse. Les deux limites d'upload cessent silencieusement de s'appliquer. C'est le pire cas : la garde a l'air présente et ne bloque plus rien.#4 / #8 — le vrai risque ici est l'injection de prompt, pas le XSS
/<script[\s\S]*?<\/script>/gine matche pas</script >(avec espace) ni</script\n>. Le corps du script fuitait donc dans le texte extrait — et ce texte alimente le prompt de Laura. Une page hostile pouvait cacher des instructions dans un<script>à balise fermante espacée et les faire passer pour du « contenu de page » auprès du modèle.Vérifié avant/après :
</script >hi evil() LEAKhi LEAK</script\n>hi evil() LEAKhi LEAK</style >ok a{} LEAKok LEAKCommentaires HTML également retirés.
#5 — durcissement, pas une faille
hrefrecevait la saisie utilisateur validée par un simple préfixe regex. Le préfixe^https?://bloquait déjàjavascript:, donc ce n'était pas exploitable. Remplacé par un parsingnew URL+ liste blanche de protocoles, qui rejette en plus les URL malformées ; tout ce qui échoue retombe en texte brut.#1 — permissions du workflow
permissions: contents: readajouté àci.yml(le workflow ne fait que builder et tester).Dependabot — lockfile uniquement
La plage déclarée
^7.14.1était déjà satisfaisante ; seules des versions vulnérables étaient figées dans le lock.react-router/react-router-dom7.18.1 → 7.18.3 (HIGH, GHSA-qwww-vcr4-c8h2, contournement CSRF en mode RSC)js-yaml→ 4.3.2 (HIGH, CPU quadratique sur!!omap)postcss→ 8.5.26 (MEDIUM, correctif incomplet de GHSA-6g55-p6wh-862q)Vérification
npm run linttsc --noEmitvitestnpm run buildnpm run security:scannpm run provenance:checknpm audit🤖 Generated with Claude Code
https://claude.ai/code/session_01AFfrQjSxUQ5EPDFNt6Qd6r