# Audit de code — PMServer (Password Manager) > Analyse statique + revue architecture au 2026-07-03. > Portée : crypto, auth, HTTP lockdown, sync, frontend, robustesse. > Basé sur la lecture du code, pas sur un pentest dynamique. Légende sévérité : 🔴 critique · 🟠 important · 🟡 moyen · 🔵 mineur / cosmétique --- ## 0. Résumé exécutif Le produit est **solide pour un gestionnaire perso offline** : chiffrement client-side AES-GCM, zero-knowledge login (le serveur ne voit jamais le master pw), défense en profondeur sur le serveur loopback (token + PID check + CSP stricte), DPAPI pour la persistance device-bound. La majorité des "bugs" trouvés cette session ont été corrigés. **MAJ 2026-07-09 : les 5 axes de durcissement ci-dessous sont tous traités.** 1. ~~🔴 Verifier = clé~~ → ✅ **découplé** (`-v2` : verifier = SHA256(keyHex+domaine)). §1.1 2. ~~🟠 Sync sans concurrence atomique~~ → ✅ **ETag/If-Match + 412 re-merge**. §2.1 3. ~~🟠 Arbitrage tombstone / horloge~~ → ✅ **timestamps UTC partout**. §2.2 4. ~~🟠 PBKDF2 GPU-friendly~~ → ✅ **Argon2id** (`argon2id-v2`). §1.2 5. ~~🟡 monofichier + zéro test~~ → ✅ **8 modules + 65 tests** (gate de build). §3.1/§3.2 --- ## 1. Sécurité ### 1.1 ✅ Verifier = matériel de clé — corrigé (schemes `-v2`) **Fait** : les schemes `pbkdf2-sha256-v2` et `argon2id-v2` transmettent `verifier = SHA256(keyHex + "pmserver/auth-verifier/v2")` au lieu de `keyHex` (`isDecoupledVerifierAlgo`/`verifierFromKeyHex`, [js/app.crypto.js](js/app.crypto.js)). Le verifier est désormais une fonction one-way de la clé → intercepter `/login` ne donne plus la clé AES. Adoption register + rotation ; comptes pré-v2 basculent à leur prochaine rotation. La clé AES reste la sortie brute du KDF (entries déchiffrables). Détails CLAUDE.md « Auth-hash schemes ».
Analyse d'origine `deriveKeyAndVerifier()` ([js/app.js](js/app.js)) dérive **un seul** output PBKDF2 et l'utilise pour DEUX rôles : - `cryptoKey` = les 32 octets bruts importés en AES-GCM (chiffre les entries) - `verifier` = **ces mêmes 32 octets en hex**, envoyés au serveur à `/login` Le serveur stocke `SHA-256(verifier)`, donc une fuite de la **DB** ne donne pas la clé (préimage SHA-256). MAIS le `verifier` transitait en clair vers `/login` = **directement la clé du vault** si le corps de requête fuit. C'est exactement ce que `-v2` corrige (verifier = one-way de la clé).
### 1.2 🟠 PBKDF2-SHA256 vs Argon2id — **corrigé (2026-07-05)** PBKDF2-SHA256 (600k) reste **GPU/ASIC-friendly** → un master pw faible tombe vite sur matériel dédié si la DB fuit. **Fait** : nouveau scheme `argon2id-v2` (Argon2id memory-hard + verifier décplé). Détails dans le CLAUDE.md « Auth-hash schemes ». - **JS pur** (`js/argon2.js`, bundle `@noble/hashes@2.2.0`), **pas WASM** : la CSP `script-src 'self'` n'accorde pas `wasm-unsafe-eval`. Vérifié contre le vecteur RFC 9106 §5.3 (test unitaire). - **Zéro Argon2 côté Delphi** (l'audit se trompait sur ce point) : le serveur ne dérive jamais la clé, il stocke/échoie juste les params (`argon2_m/t/p`) et SHA256-wrappe le verifier comme tout `-v2`. - Params OWASP `m=19 MiB, t=2, p=1` (~0.65 s/unlock), stockés par compte. - **Adoption** : register + change-master-pw (nouveau défaut). Comptes existants restent PBKDF2 jusqu'à rotation. Passer à Argon2id re-chiffre tout le vault (la clé dérivée change) — c'est le flow rotation existant. - Tests : 8 tests crypto Argon2 (vecteur RFC, branche KDF, contrat de params register↔login, sensibilité aux params). 42/42. **Validé runtime** : un compte ayant tourné sa master pw affiche `hash_algo=argon2id-v2` (m=19456, t=2, p=1) et se reconnecte/déchiffre. ✅ **Async (2026-07-05)** : `deriveKeyBytes` utilise `argon2idAsync` (bundle re-vendé pour l'exposer) — cède la main à l'event loop pour que le spinner s'anime au lieu de figer ~0.65 s. Parité sync/async testée sur le vecteur RFC 9106. **Plus rien d'ouvert sur §1.2.** ### 1.3 🟡 Métadonnées en clair Documenté mais à rappeler pour un futur modèle de menace : - `entry_attachments` : `filename`, `mime`, `size_bytes` **non chiffrés** - `users.avatar_b64` : image **non chiffrée** (cosmétique, assumé) - `vault_entries` : ~~`username`, `site`, `title`, `tags`, `template`~~ **chiffrés (username/site/title/tags 2026-07-09 ; template 2026-07-11)** ; `folder`, `kind` restent en clair (folder = requête serveur de réassignation ; kind = 2 valeurs, ~0 entropie). **`username` + `site` + `title` + `tags` chiffrés au repos (✅ 2026-07-09)** : colonnes `_enc/_iv` (AES-GCM sous la clé du vault). Clé de l'approche : recherche/tri sont **côté client** → on déchiffre au `loadEntries` en mémoire, donc AES-GCM plein (IV aléatoire), pas de searchable-encryption. Choke-point `withEncryptedMeta` sur tous les writes ; `decryptEntryMeta` au load ; migration `migrateMetadataAtRest` (live + corbeille) ; rotation re-chiffre les 4. Validation serveur « Site required » retirée + `?q=` neutralisé (LIKE inutile sur ciphertext). Détails CLAUDE.md « Entry payload ». **Validé runtime le 2026-07-09** : après rebuild + unlock du compte `test`, la base montre **0 en clair** sur les 4 champs (username/site/title/tags) pour ce compte, `_enc` remplies, affichage/ recherche/favicons OK. La migration est **par utilisateur** (tourne au unlock sur les entries du compte connecté) — un compte non connecté garde son clair jusqu'à sa prochaine connexion (comportement normal, pas une régression). **`template` chiffré au repos (✅ 2026-07-11)** : même pattern (`template_enc`/`template_iv`, ajouté à `ENCRYPTED_META_FIELDS`). Migration vérifiée (0 clair restant, 2 comptes) → refs colonne clair retirées du code + `AddColumnIfMissing` supprimés (template, et tags/title qui étaient revenus en fantômes après le drop) + CREATE TABLE corrigé (site/username NOT NULL cassait une DB neuve). Guard dans `withEncryptedMeta` : les re-ships partiels (add-tag, move-to-folder) omettent `template` → le serveur le préserve via `LHasTemplate` (clé absente), pas de wipe. 3 tests unitaires. **Pièces jointes : le contenu est déjà chiffré** (AES-GCM, `encryptBlobBytes`) ; seuls `filename`/`mime`/`size_bytes` restent en clair (pour lister sans tout déchiffrer). Résiduel : `folder`, `kind`, métadonnées d'attachments, nombre de lignes, timestamps. ### 1.4 ✅ Sync password faible — corrigé (2026-07-09) Le snapshot WebDAV n'est protégé que par le sync password. **Fait** : `syncSetEncPwdFlow` exige maintenant **≥12 chars ET `computeStrength ≥ 50`** (réutilise `computeStrength`, pas de dép zxcvbn). Le maillon faible est fermé. ### 1.5 🔵 Points positifs (à conserver) - ✅ AES-GCM 256 (AEAD, intégrité incluse) — pas de mode ECB/CBC nu - ✅ IV aléatoire par blob (`crypto.getRandomValues(12)`) — pas de réutilisation - ✅ Zero-knowledge : le master pw plaintext ne quitte jamais le client - ✅ CSP stricte (`default-src 'self'; img-src 'self' data:`) — bloque l'exfiltration même en cas d'injection - ✅ Serveur loopback + token + PID-on-socket check (defense in depth) - ✅ DevTools / menu contextuel Edge désactivés - ✅ Rendu via `textContent` / `el()` (pas d'`innerHTML` sur données user) → surface XSS minimale - ✅ Clipboard sécurisé (exclusion clipboard history + auto-clear 30s) - ✅ Recovery code à usage limité (5), consommé, supprimé après rotation --- ## 2. Bugs / risques latents (non encore observés) ### 2.1 ✅ Sync : lost update — corrigé (ETag/If-Match) **Fait** : concurrence optimiste implémentée. `runSyncNow` capture l'ETag au `GET`, l'envoie en `If-Match` au `PUT` ([js/app.sync.js](js/app.sync.js)) ; un **412** → re-pull + re-merge + re-push (borné à 3). Serveur sans ETag → etag vide → pas d'If-Match → fallback last-write-wins. Détails CLAUDE.md « Sync ». ### 2.2 🟠 Arbitrage tombstone sensible à l'horloge — **corrigé (2026-07-04)** `applyRemoteSnapshot()` compare `entry.updated_at > tombstone.deleted_at` pour décider résurrection vs suppression. **Bug réel trouvé** (pire que décrit) : deux sources d'horloge coexistaient **sur un même device**. `created_at`/`updated_at` des entries étaient écrits via Delphi `FormatDateTime(..., Now)` = **heure locale**, tandis que `deleted_at` (soft-delete + tombstones) venait de SQLite `CURRENT_TIMESTAMP` / `datetime('now')` = **UTC**. L'arbitrage comparait donc un `updated_at` local à un `deleted_at` UTC → décalage = l'offset UTC de la machine, **même en solo mono-device** (pas seulement entre devices décalés). **Fix** : helper `NowUTC` / `NowUTCStr` dans `PM.Database`, substitué à `FormatDateTime(..., Now)` à tous les sites d'écriture d'entries/attachments ([PM.Handler.Entries.pas](delphi-backend/Handlers/PM.Handler.Entries.pas) create/update/bulk, [PM.Handler.Attachments.pas](delphi-backend/Handlers/PM.Handler.Attachments.pas)). Tout est désormais UTC (les paths SQLite l'étaient déjà). Côté JS aucun changement nécessaire : `buildSyncSnapshot` utilise `toISOString()` (UTC) et l'arbitrage compare deux valeurs du **même** fuseau → `Date.parse` applique le même offset local aux deux, ordre relatif préservé. **Rows existantes** : décision « laisser se soigner » — les anciennes lignes gardent leur `updated_at` local jusqu'à leur prochain edit (qui le réécrit en UTC). Exposition étroite pour un vault solo ; pas de migration destructive (un shift par l'offset courant serait irréversible et approximatif : DST / changement de fuseau historique). **Reste (optionnel)** : compteur logique (Lamport) pour les cas d'égalité stricte ; incohérence de **format** pré-existante (serveur `'yyyy-mm-dd hh:nn:ss'` vs export JS `toISOString()` avec `T…Z`) sur les rares rows sans `created_at`. ### 2.3 🟡 `catch (_) {}` silencieux en cascade Nombreux `catch (_) {}` dans `applyRemoteSnapshot`, `buildSyncSnapshot`, restauration d'attachments, création de folders. Une erreur réseau/serveur transitoire est avalée → l'entry/folder/attachment est silencieusement sauté. Le garde-fou "failed count → abort push" atténue la perte côté entries, mais **pas** côté attachments ni folders. **Recommandation** : logger chaque `catch` (au moins `console.warn` + compteur), et étendre le garde-fou "n échecs → warn utilisateur" aux attachments. ### 2.4 🟡 Pas de pagination serveur sur `GET /entries` Tout le vault est chargé + déchiffré à chaque unlock. Sur un gros vault (5k-10k entries), `buildSyncSnapshot` (déchiffre chaque entry + fetch attachments un par un) et `computeHealthCache` deviennent lents (O(n) séquentiel, `await` en boucle). Acceptable pour usage perso (< 500 entries), problématique au-delà. **Recommandation** : batcher les déchiffrements (Promise.all par lots), et si besoin paginer côté serveur pour la vue grille. ### 2.5 ✅ 500 au lieu de 401 sur session expirée — corrigé (2026-07-09) `Authenticate` écrit 401 puis `raise ESessionRejected` → le `try/except` global de `PM.HTTPServer` réécrit un **500**. Pré-existant, tous les handlers concernés. Cosmétique (le client voit un non-2xx) mais brouille le debug. **Recommandation** : attraper `ESessionRejected` spécifiquement dans le dispatcher et ne pas réécrire la réponse déjà envoyée. ### 2.6 🔵 CSV re-import crée des doublons Documenté/assumé (les lignes CSV sans uuid mint un nouvel uuid à chaque import). Le dedup uuid ne couvre que le JSON. Acceptable si documenté à l'utilisateur. ### 2.7 🔵 `navigator.clipboard` fallback échoue silencieusement si caché Quand le Bridge Delphi est absent, le fallback `navigator.clipboard. writeText` échoue si le document n'a pas le focus (WebView caché). Cas rare (le Bridge est quasi toujours actif en prod) mais le `catch (_) {}` masque l'échec → l'utilisateur croit avoir copié. --- ## 3. Qualité de code / maintenabilité ### 3.1 🟡 `app.js` monofichier ~14 000 lignes Tout le frontend en un seul fichier. Difficile à naviguer, à tester, et un edit malencontreux casse tout le parse (déjà arrivé cette session : une déclaration de fonction supprimée → app entièrement morte, découvert seulement au runtime). **En cours (2026-07-05)** : découpage incrémental en fichiers classic-script chargés dans l'ordre via `