Aller au contenu

PD-275 — Revue Sécurité

Résumé

Critère Statut Commentaire
Forbidden patterns ✅ Conforme Le plan d'implémentation privilégie l'usage de l'ORM, évitant a priori le SQL brut.
Injection SQL ✅ Conforme L'usage de TypeORM et des requêtes paramétrées prévient efficacement les injections SQL.
Auth/Authz ✅ Conforme Mécanismes robustes (RolesGuard, double-check service, dérivation revokedBy du JWT).
Fuite données sensibles ⚠️ Réserves [S-01] La réponse de l'API de révocation peut exposer l'ID de l'administrateur.
Validation ⚠️ Réserves [S-02] Validation de la configuration FINALITY_DEPTH manquante. [S-03] Normalisation d'adresse non garantie.

Verdict : ⚠️ RÉSERVES

Le plan d'implémentation est globalement très robuste et démontre une excellente maturité en sécurité, notamment sur la gestion de la concurrence et la prévention des TOCTOU.

Les réserves émises sont mineures ou concernent des points de renforcement qui, une fois adressés, permettront d'atteindre un niveau de confiance maximal.

Audit des forbidden patterns

Le plan d'implémentation est conforme aux bonnes pratiques en s'appuyant systématiquement sur l'ORM TypeORM pour les interactions avec la base de données.

Pattern interdit Recherché dans le plan Trouvé
SQL brut (raw) Utilisation de queryRunner.query() avec concaténation ❌ Non
Désactivation des gardes Contournement de OidcJwtAuthGuard ou RolesGuard ❌ Non
Secrets en clair Stockage de clés ou secrets dans le code ou la configuration ❌ Non

Tentatives de bypass

Attaque Résultat attendu (selon le plan) Commentaire
Injection de revokedBy dans le payload Rejeté (400 ERR-REVOKEDBY-SPOOFING) Le controller valide l'absence du champ et dérive revokedBy du JWT. Le DTO ne contient pas le champ.
Révocation par un utilisateur non autorisé Rejeté (403 ERR-REVOKE-UNAUTHORIZED) Le RolesGuard sur l'endpoint bloque l'appel.
TOCTOU : submit pendant une revoke concurrente Rejeté ou sérialisé correctement Le verrou pessimiste SELECT ... FOR UPDATE sur signer_registry garantit la sérialisation des opérations. Le double-check dans le service submitBatch couvre la race condition.
Usurpation de statut (bypass de révocation) Rejeté (ERR-SIGNER-REVOKED) La source de vérité (signer_registry) est systématiquement vérifiée sous verrou transactionnel avant toute soumission.
Finalisation d'un batch non confirmé Rejeté (ERR-FINALITY-INSUFFICIENT) Le FinalityGuardService bloque la transition si confirmation_count < FINALITY_DEPTH.

Vulnérabilités identifiées

ID Description Gravité Fichier / Composant
S-01 Fuite potentielle de l'ID de l'administrateur : L'endpoint de révocation POST /anchor/signers/:address/revoke retourne un objet contenant revokedBy, qui est le sub du JWT de l'administrateur. Cet ID pourrait être une information personnelle (email, UUID interne) qu'il n'est pas souhaitable d'exposer publiquement. MINEUR C7 — SignerController
S-02 Absence de validation de la configuration FINALITY_DEPTH : Le plan spécifie que FINALITY_DEPTH est lu depuis la configuration, mais ne contractualise pas une validation stricte de cette valeur au démarrage. Une valeur de 0 ou négative, issue d'une erreur de configuration, pourrait permettre une finalisation instantanée des batches, annulant la garde de finalité. MAJEUR C5 — FinalityGuardService
S-03 Risque de désynchronisation par absence de normalisation d'adresse : Le plan identifie le besoin de normalisation EIP-55 (checksum) comme une "recommandation" (HT-07) mais ne l'intègre pas comme une exigence contractuelle dans le service. Sans normalisation systématique à l'écriture et à la lecture, une même adresse avec des casses différentes (ex: 0xabc... vs 0xAbC...) pourrait être traitée comme deux signers distincts, permettant un bypass de la révocation. MAJEUR C4 — SignerRegistryService

Recommandations

  1. [S-01] Masquer l'ID de l'administrateur dans la réponse API :

    • Créer un DTO de réponse pour l'endpoint de révocation qui omet le champ revokedBy.
    • Alternativement, utiliser l'decorator @Exclude() de class-transformer sur le champ revokedBy de l'entité SignerRegistry pour ne jamais l'exposer via les controllers.

  2. [S-02] Rendre la validation de FINALITY_DEPTH non négociable :

    • Dans le FinalityGuardService ou via un validateur de configuration global (ex: class-validator sur l'objet de config), ajouter une assertion au démarrage de l'application.
    • Cette assertion doit garantir que FINALITY_DEPTH est un nombre entier strictement positif (>= 1). Si la validation échoue, l'application doit refuser de démarrer.

  3. [S-03] Intégrer la normalisation EIP-55 comme une contrainte d'intégrité :

    • Dans SignerRegistryService, toutes les méthodes recevant une adresse en paramètre (findByAddress, revokeSigner, etc.) doivent systématiquement la normaliser en EIP-55 (ex: via ethers.utils.getAddress(address)) avant de l'utiliser dans une requête.
    • La colonne address de la table signer_registry doit être contractuellement définie pour ne stocker que des adresses normalisées EIP-55. La migration devrait inclure une contrainte CHECK si possible, ou a minima la logique applicative doit être blindée.