+25
−1
+41
−2
+209
−8
File changed.
Preview size limit exceeded, changes collapsed.
+4
−2
Loading
Sécurité : correctifs d'audit, et second facteur TOTP sur les comptes
Deux sujets dans un seul commit, et il faut le dire : les correctifs de l'audit
et le second facteur se recouvrent sur six fichiers — le garde de session, les
types de jeton, le schéma, la documentation. Le TOTP s'appuie directement sur la
révocation de session introduite par le correctif M3. Les séparer aurait demandé
un découpage par section, avec un commit intermédiaire dans un état que je ne
pouvais pas compiler pour le vérifier.
═══════════════════════════════════════════════════════════════════
PARTIE 1 — Audit de sécurité : 4 constats hauts, 6 moyens, 6 faibles
═══════════════════════════════════════════════════════════════════
H1 — XSS stocké via la GED
Le type MIME stocké était celui DÉCLARÉ PAR LE CLIENT, et il était renvoyé tel
quel en affichage direct. Déposer un fichier HTML suffisait à faire exécuter du
script sur l'origine de l'application chez quiconque ouvrait le document. Le
cookie est httpOnly, donc illisible par ce script — mais il part tout seul sur
chaque requête : le script agissait au nom de sa victime sans jamais voir son
jeton. Un concepteur obtenait ainsi d'un administrateur les actions que son
propre rôle lui refuse.
Trois barrières cumulatives (common/file-serving.ts), parce qu'aucune ne suffit
seule : liste blanche à l'entrée, téléchargement forcé sauf types sûrs, et
`nosniff` qui empêche le navigateur de redeviner le type — sans quoi la première
serait contournable. La politique `sandbox` s'y ajoute pour neutraliser ce qui est
DÉJÀ en base : les fichiers déposés avant ce durcissement n'ont pas de type fiable.
H2 — XSS via logo SVG, atteignable sans authentification
`image/svg+xml` figurait dans la liste blanche des logos sous la mention « rien
d'exécutable ». C'est faux : un SVG accepte <script>, <foreignObject> et des
gestionnaires d'événements. La liste des images d'exercice l'excluait déjà pour
cette raison exacte — les deux listes du même dépôt se disaient « rien
d'exécutable » et une seule le respectait. Servi par une route volontairement
ouverte, il donnait un XSS stocké sans session.
H3 — Aucun plafond sur /auth/login
Le garde de plafonnement existait, testé, posé sur le bac à sable et les routes
de compte. Pas sur l'authentification. Deux conséquences : force brute libre, et
déni de service par le processeur — chaque essai coûtant un bcrypt à 12 tours,
quelques dizaines de requêtes concurrentes saturaient une API mono-processus.
10 essais par quart d'heure et par adresse, 5 par compte visé. Les deux plafonds
répondent à deux attaques distinctes et aucun ne remplace l'autre ; le second ne
peut pas servir à bloquer autrui puisqu'il s'ajoute au premier.
H4 — Le scénario partait chez les joueurs
`exerciseRoom` est partagée : joueurs et écrans de salle y sont aussi. Le rappel
d'appel, l'avancement des injects et le journal d'événements y étaient diffusés.
Le code portait pourtant « rappel côté animateur : aucune diffusion aux joueurs »
à la ligne PRÉCÉDANT la diffusion, et le type partagé disait « non transmis aux
joueurs ».
Les clients joueurs ne s'y abonnaient pas, donc rien ne s'affichait. Ne pas
afficher n'est pas ne pas transmettre : les trames arrivaient et se lisaient dans
les outils de développement. Pour un public d'exercices CYBER, supposer que
personne ne les ouvrira n'était pas raisonnable — et un joueur qui lit le scénario
à l'avance annule l'exercice.
Salle `ex:{id}:staff`, que les sockets d'animation rejoignent EN PLUS de la salle
commune : aucune double émission, et ce qui est légitimement commun (horloge,
presse, réseaux sociaux, main courante — vérifié composant par composant) continue
d'y passer.
Moyens
M1 Aucun en-tête de sécurité. Écrits à la main plutôt qu'avec `helmet` : quatre
en-têtes ne justifiaient pas une dépendance de plus dans un dépôt qui assume
sa frugalité. L'empreinte du script en ligne d'index.html est CALCULÉE au
démarrage depuis le fichier servi — une constante en dur aurait dérivé au
premier changement de page, et une empreinte fausse casse l'application en
silence. Fichier introuvable : la directive est omise plutôt que fausse.
M2 `trust proxy` inactif. Le défaut se payait à l'envers : derrière un proxy,
tous les visiteurs partageaient un seau, soit trois bacs à sable par heure
pour la planète. TRUSTED_PROXIES, avec les deux erreurs symétriques
documentées au README.
M3 Aucune révocation de session. Réinitialiser un mot de passe — le geste qu'on
fait quand on soupçonne une compromission — laissait la session de l'attaquant
vivre huit heures. Compteur `sessionEpoch` dans le jeton. Les jetons émis
avant ne le portent pas et restent valides : un correctif de sécurité ne doit
pas déconnecter tout le monde d'un coup.
M4 SMTP sans TLS imposé. STARTTLS opportuniste : un attaquant réseau retirait la
capacité de la réponse EHLO et obtenait en clair les identifiants SMTP et les
liens de réinitialisation. `requireTLS` + TLS 1.2 minimum.
M5 Objets des messages joueur→animation diffusés à tous les joueurs. Même
correctif que H4.
M6 Journal d'audit d'instance et demandes d'inscription conservés sans limite de
durée, données personnelles comprises — y compris les demandes REJETÉES, qui
n'ont ouvert aucun compte. Durées définies et documentées.
Faibles
L1 Confinement de chemin par préfixe : un répertoire frère partageant le préfixe
passait. Non exploitable (clés UUID, chemins de kit venant du dépôt), mais
c'était une traversée en attente d'une entrée utilisateur.
L2 Secrets de 16 caractères acceptés pour du HS256 et de l'AES-256. Portés à 32.
L3 Troncature silencieuse à 72 octets par bcrypt, alors que 200 caractères
étaient acceptés.
L4 Listes blanches de routes comparées par `endsWith`, donc par sous-chaîne.
Rien d'exploitable — un garde s'exécute après le routage — mais toute route
ajoutée plus tard et terminant par le même suffixe héritait de l'exemption en
silence. Comparaison exacte : la classe disparaît.
L5 NON TRAITÉ. multer 1.x et bcryptjs 2.4.3 : montées majeures impossibles à
éprouver sans exécuter les tests. Consigné dans DEPENDENCIES.md avec la
décision à prendre.
L6 APP_URL retombait sur localhost, produisant des liens morts sans le dire.
Recommandation appliquée : la passerelle temps réel ne vérifiait pas la
suspension d'organisation. Une organisation suspendue gardait ses connexions, et
une nouvelle réussissait.
Trouvés en corrigeant
- `LoginDto.password` sans borne supérieure.
- `CreateUserDto` acceptait 8 caractères là où le libre-service en exigeait 10 :
un compte créé par un administrateur pouvait être plus faible que celui que
l'intéressé se serait choisi. D'où la politique centralisée, minimum partagé
avec l'interface pour qu'un écran n'annonce plus une règle différente de celle
qui est appliquée.
- SIX dépendances déclarées mais absentes de DEPENDENCIES.md, contrairement à la
politique du dépôt.
- La GED n'avait AUCUNE section de sécurité. Ce n'est probablement pas étranger
au fait qu'elle portait le constat le plus grave.
Confirmé sain, sans modification
Zéro injection SQL possible (une seule requête brute, `SELECT 1`). Zéro puits XSS
sur 16 800 lignes de front — pas un `dangerouslySetInnerHTML`, pas un `eval`.
Cloisonnement verrouillé par construction : les 33 modèles portant `tenantId`
sont tous couverts, aux deux exceptions vérifiées comme délibérées. Jetons à usage
unique corrects de bout en bout. Chiffrement au repos bien fait.
═══════════════════════════════════════════════════════════════════
PARTIE 2 — Second facteur TOTP (RFC 6238)
═══════════════════════════════════════════════════════════════════
Optionnel pour tous, OBLIGATOIRE pour l'administration : INSTANCE_ADMIN dans
l'organisation système, TENANT_ADMIN dans une organisation cliente. Ce sont les
deux rôles qui créent des comptes, changent des rôles et effacent en cascade.
L'obligation se lit sur TOUTES les appartenances
Le point important. Le jeton ne désigne qu'une organisation, et un compte peut en
avoir plusieurs : ne regarder que l'active aurait laissé un administrateur
d'organisation se soustraire à l'obligation en basculant vers une organisation où
il n'est qu'animateur. Un test épingle ce cas.
Deux niveaux d'exigence
Immédiat pour l'administration d'instance : peu nombreuse, elle ne conduit pas
d'exercice, rien ne justifie de différer. Délai de grâce de 14 jours (réglable en
base) pour l'administration d'organisation : un administrateur peut être en pleine
conduite d'exercice, téléphone hors de portée, et le bloquer à cet instant serait
un défaut, pas une protection.
Le mot de passe seul n'ouvre rien
`POST /auth/login` ne pose AUCUN cookie de session quand un second facteur est
actif : seulement un jeton d'attente de cinq minutes, dans un cookie DISTINCT.
Trois barrières le séparent d'une session, plutôt qu'un contrôle unique qu'on
pourrait oublier : ce n'est pas le cookie que le garde lit, le jeton porte un
marqueur `stage` que le garde refuse explicitement, et il expire vite.
L'échéance voyage dans le JETON, pas relue à chaque requête. C'est correct parce
que toute promotion de rôle incrémente le compteur de révocation, donc force une
nouvelle signature : la valeur ne peut pas se périmer dans le sens permissif.
Anti-rejeu
La tolérance de ± 1 pas laisse un code valable jusqu'à 90 s — assez pour le
rejouer. Le dernier pas accepté est enregistré et tout pas antérieur ou égal
refusé, même si le code est mathématiquement correct.
Trois voies de secours
Rendre une protection obligatoire sans prévoir sa reprise en main, c'est préparer
une panne.
1. Codes de secours : 8 codes à usage unique, empreinte seule stockée, alphabet
sans caractères ambigus (ils se recopient à la main). Consommation ATOMIQUE
par écriture conditionnelle : deux requêtes simultanées avec le même code ne
peuvent pas réussir toutes les deux.
2. Réarmement par la console d'instance, MOTIF OBLIGATOIRE, tracé au journal
persistant. Le délai de grâce repart à zéro — délibérément : sinon une échéance
déjà dépassée rebloquait la personne dans la seconde et le geste ne servait à
rien.
3. Commande d'exploitation pour l'administrateur d'instance, qui n'a personne
au-dessus de lui. Elle demande un accès serveur et base, précisément ce qui en
fait un privilège acceptable — cet accès permet déjà tout. Elle avertit et
exige --confirm avant d'écrire.
Chacune révoque toutes les sessions du compte : on vient de baisser son niveau
d'authentification.
Le second facteur ne peut pas être retiré quand il est imposé. Sinon l'obligation
ne vaudrait rien — il suffirait de l'activer une fois puis de le retirer.
Dépendances
otplib 12.0.1 et qrcode 1.5.4, épinglées, licences et niveau de maintenance
documentés. otplib n'a pas été publiée depuis 2021 : la norme TOTP est figée, donc
ce n'est pas alarmant, mais c'est consigné avec la porte de sortie (otpauth, la
primitive étant isolée dans un seul fichier testé). Le QR est rendu dans le
NAVIGATEUR, en data-URI dans une <img> : rien ne sort de l'instance, et le dépôt
ne gagne pas son premier dangerouslySetInnerHTML.
Trois défauts trouvés en écrivant, et corrigés
- Dans mon propre code : `verifyTotp` recevait l'instant pour calculer le pas,
mais otplib vérifiait contre l'horloge réelle. Les deux pouvaient diverger et
l'anti-rejeu aurait enregistré un pas qui n'était pas celui qui avait répondu.
L'instant est désormais passé explicitement.
- Un verrou possible : une session en observation ne peut pas s'enrôler (lecture
seule) et n'aurait pas pu sortir. Non atteignable — un administrateur
d'instance n'obtient aucune session avant enrôlement — mais /instance/exit est
ajouté à la liste blanche par précaution.
- Du code mort que j'avais écrit : `requirementFor` et `hashesEqual`, ce dernier
testé mais jamais appelé, avec un commentaire sur les attaques temporelles qui
ne s'appliquait à rien.
═══════════════════════════════════════════════════════════════════
VÉRIFICATIONS
═══════════════════════════════════════════════════════════════════
NON EXÉCUTÉES. Ni node ni pnpm n'étaient disponibles, node_modules n'était pas
installé. Aucun test, aucun typecheck, aucun lint. C'est le point le plus
important de ce message : ce commit contient du code qui n'a jamais tourné.
Relecture statique faite : constructeurs alignés dans les harnais existants
(AuthService en avait un de moins), longueurs de ligne contrôlées pour prettier,
39 entrées i18n ajoutées avec vérification programmatique qu'aucune chaîne ne
manque, recoupement automatisé des dépendances contre DEPENDENCIES.md.
Deux points s'appuient sur une API d'otplib non exécutée ici : `clone()` et
`checkDelta()`. Un échec y serait immédiat et bruyant au démarrage, pas une
faille silencieuse — mais c'est la première chose à confirmer.
À faire avant de lancer :
pnpm install
pnpm --filter @cythin/api exec prisma migrate deploy && prisma generate
pnpm test && pnpm typecheck && pnpm lint
Le lockfile n'est PAS à jour : otplib et qrcode sont déclarées, pas installées.
Trois changements peuvent empêcher le démarrage en production :
- JWT_SECRET doit faire 32 caractères (contre 16) — échec au démarrage sinon.
- APP_URL devient obligatoire — même effet.
- TRUSTED_PROXIES à 0 par défaut : derrière un proxy, les plafonds restent
globaux tant qu'il n'est pas renseigné.
À rejouer à la main : téléverser un document de chaque type accepté et refusé ;
vérifier dans un onglet joueur qu'aucun call:reminder n'arrive plus ; activer le
TOTP sur un compte, consommer un code de secours, et vérifier qu'un administrateur
d'organisation au délai expiré ne voit plus que l'écran d'enrôlement.
Co-Authored-By: Claude (RCA)