perun/AUDIT.md

Audit de reprise — Perun for DCS World

Revue menée pour décider d'une reprise du projet (upstream szporwolik/perun, archivé le 2026-03-29, licence MIT). Fork de travail : DaKerboul/perun, miroir Gitea : git.kerboul.me/kerboul/perun.

Méthode. Revue à 5 casquettes (Architecte, Sécurité, Backend .NET, DCS/Lua, DevOps/DBA). Périmètre audité : le code réellement écrit par le projet — hook Lua (01_DCS), app C# WinForms (02_Windows_App), wrapper C++ (03_*, hors arbre Lua 5.1.5 vendoré), schéma MySQL (04_MySQL), exemple PHP (05_Misc). Les ~35 .c/.h de lua-5.1.5/ sont du Lua amont vendoré et ne sont pas audités. Date : 2026-06-11.

---

Synthèse — verdict

Projet fonctionnellement riche et bien pensé sur le fond (modèle de données propre, couverture événementielle DCS complète, multi-instances, intégrations SRS/LotATC). Mais dette de sécurité critique et code applicatif daté (monolithes, threading artisanal, zéro test, zéro CI, Windows/.NET Framework uniquement). C'est une bonne base à reprendre, à condition de traiter les points P0 avant toute remise en production.

#	Domaine	Gravité	Sévérité
S1	Injection SQL via données joueur (app C#)	Critique	🔴 P0
S2	Port TCP sans authentification, bind 0.0.0.0	Critique	🔴 P0
S3	XSS stocké dans l'exemple PHP	Élevé	🟠 P1
S4	Mot de passe MySQL stocké en clair	Élevé	🟠 P1
S5	PII (IP, UCID) sans rétention ni consentement (RGPD)	Élevé	🟠 P1
B1	Threading non synchronisé (buffer partagé)	Élevé	🟠 P1
B2	ExecuteReader pour des INSERT/UPDATE, 1 connexion/frame	Moyen	🟡 P2
A1	Monolithes, dynamic, aucun test, aucune CI	Moyen	🟡 P2
L1	Fuite de variables globales dans le hook DCS	Moyen	🟡 P2
D1	Lua 5.1.5 vendoré dans le repo, pas de build reproductible	Moyen	🟡 P2
D2	Pas de FK, dépendance à STRICT_TRANS_TABLES off, pas de migrations	Faible	🟢 P3

---

1. Architecte / Lead — structure & dette

Points forts

• Découpage fonctionnel clair en 5 dossiers numérotés, lisible d'emblée.
• Séparation nette des responsabilités : collecte (Lua) → transport (TCP/DLL) → persistance (C#) → restitution (PHP). Le protocole de trames (IDs 1/2/3/50…101) est documenté dans le README.
• Multi-instances supporté de bout en bout (champ instance partout).

Points faibles

• Monolithes. DatabaseController.SendToMySql fait ~290 lignes avec un switch géant mêlant construction SQL, mapping métier et logging (01_Classes/DatabaseController.cs:12-304). Idem TCPController.StartListen (boucles imbriquées sur ~170 lignes).
• dynamic partout pour le JSON entrant (DatabaseController.cs:25, TCPController.cs:121) : aucune validation de schéma, accès TCPFrame.payload.x qui lèvent au moindre champ manquant → exceptions au lieu d'un rejet propre.
• Plateforme verrouillée : VS2017, .NET Framework 4.8, WinForms → Windows uniquement, fin de vie. Or le serveur DCS est Windows, mais l'app de persistance n'a aucune raison d'y être clouée (elle ne parle que TCP + MySQL).
• TODO laissé en dur (commentaire polonais) : DatabaseController.cs:92 « TUTAJ DODAC CATCH TBD » → gestion d'erreur inachevée.
• Aucun test, aucune CI, dérive de version (cf. §4 L-version).

À faire

• Extraire la construction SQL dans une couche dédiée (un handler par type de trame) + DTO typés à la place de dynamic.
• Cibler .NET 8 + un worker headless multiplateforme ; garder l'UI WinForms en option (ou la remplacer par un petit panneau web/CLI).
• Introduire des tests unitaires (parsing de trames, génération SQL) et un pipeline CI.

2. Sécurité (AppSec) — bloquant pour la prod

S1 — Injection SQL via données contrôlées par le joueur. 🔴 L'app paramètre certaines valeurs (@PAR_*) mais en concatène des dizaines d'autres directement dans le SQL, dont des champs que n'importe quel joueur maîtrise (UCID, nom, hash de mission, IP, datetime, tous les compteurs ps_*) :

• DatabaseController.cs:128-132 (chat) : ucid, missionhash, all, datetime.
• DatabaseController.cs:159-165 (stats) : stat_ucid, stat_missionhash et ~30 valeurs stat_data_perun.ps_* injectées telles quelles.
• DatabaseController.cs:175-177 (login) : login_ucid, login_ipaddr, login_datetime.
• DatabaseController.cs:50-51, 77-89, 205-206 : instance/type concaténés.

Un UCID/nom forgé (ou un module client modifié) permet l'exfiltration ou la destruction de la base. C'est le défaut n°1 à corriger. → Tout passer en requêtes paramétrées (MySqlParameter), sans exception.

S2 — Listener TCP non authentifié, exposé. 🔴 new TcpListener(IPAddress.Any, intListenPort) (TCPController.cs:51) écoute sur toutes les interfaces, sans authentification ni allowlist. Couplé à S1, n'importe quel hôte joignant le port (48621 par défaut) injecte des trames arbitraires → compromission complète de la base. → Par défaut bind 127.0.0.1 (hook et app sont quasi toujours sur la même machine), + secret partagé/HMAC sur les trames, + allowlist d'IP.

S3 — XSS stocké (PHP). 🟠 05_Misc/05_PHP_Example/index.php réinjecte en HTML des données joueur sans échappement : message de chat (:110), nom (:92, :109, :143), contenu d'événement (:127). Un joueur dont le nom vaut <script>…</script> exécute du JS dans le navigateur de l'admin. Les requêtes elles-mêmes sont statiques (pas de SQLi côté PHP), le risque est l'XSS. → htmlspecialchars() systématique sur toute sortie ; corriger aussi le HTML invalide (<h1> fermé par </h2> ligne 44).

S4 — Identifiants MySQL en clair. 🟠 Le mot de passe est stocké en String dans les user settings .NET (02_Forms/form_Main.cs:119, clé MYSQL_Password de app.config) → écrit en clair dans user.config. → Chiffrer via DPAPI (ProtectedData) ou déléguer à un gestionnaire de secrets ; a minima ne jamais journaliser la chaîne de connexion.

S5 — Données personnelles (RGPD). 🟠 Le hook collecte et stocke adresses IP et UCID des joueurs (Perun-hook.lua:364-366 → pe_DataPlayers_lastip, pe_LogLogins_ip), sans politique de rétention ni information des joueurs. → Définir une durée de rétention + purge, anonymiser/hacher l'IP si non nécessaire, documenter (mention serveur + Discord).

3. Backend / .NET — qualité & robustesse

B1 — Threading artisanal non synchronisé. 🟠 Le thread TCP écrit dans Globals.arrMySQLSendBuffer (tableau fixe) pendant que le thread d'envoi le lit, sans verrou (TCPController.cs:130-142). Buffer plein = paquets silencieusement perdus (:139-141), scan linéaire O(n) par paquet. Accès concurrents → corruption/race. → Remplacer par une BlockingCollection<T>/Channel<T> thread-safe et bornée.

B2 — Accès base inefficace. 🟡

• ExecuteReader() utilisé pour exécuter des lots d'INSERT/UPDATE (DatabaseController.cs:217) : devrait être ExecuteNonQuery().
• Une nouvelle MySqlConnection ouverte/fermée par trame (:34, :292) : pas de réutilisation du pool, surcoût réseau par paquet.
• Tout est synchrone bloquant (pas d'async/await). → Connexion/pool réutilisé, requêtes asynchrones, ExecuteNonQueryAsync.

Autres

• Dépendance Newtonsoft.Json → migrable vers System.Text.Json.
• Gestion d'erreurs par numéros MySQL en dur (:246-279) : utile mais fragile, à compléter (le catch manquant signalé en :92).

4. DCS / Lua / Intégration

Points forts

• Couverture événementielle DCS très complète : kill (avec catégorisation PvP/AI), friendly fire, crash, eject, takeoff/landing (airfield/ship/FARP), multicrew, change_slot, connect/disconnect, chat, MOTD. C'est le vrai actif du projet (Perun-hook.lua:725-879).
• Comptage de stats maison car les stats natives DCS sont peu fiables (choix assumé et pertinent).

Points faibles

• L1 — Fuite de variables globales 🟡 : plusieurs variables sont assignées sans local dans l'environnement hook (privilégié et partagé) — ex. _temp_killers/_temp_event_type (:754-756), _master_type/_master_slot/ _sub_slot (:813), _temp_airfield (:849). Risque de collision avec d'autres hooks installés sur le serveur.
• L-version — dérive de version : la version du hook est codée en dur "v0.12.1" (:30) et vit séparément de la version de l'app (Globals.VersionPerun). → source unique de vérité (tag git → injecté au build).
• Coût par frame : onSimulationFrame fait du table.concat/JSON à chaque frame ; sur serveur chargé, surveiller le budget temps (déjà mesuré en µs dans les logs — bon réflexe à conserver/exposer).
• Dépendance à une DLL compilée (perun.dll, issue de 03_Perun_Lua_Wrapper) livrée hors repo : reproductibilité du build à fiabiliser (cf. D1).
• TCP en clair, pas de TLS (acceptable en loopback, à revoir si distant).

À faire

• local sur toutes les temporaires ; passe luacheck.
• Versionner hook + app depuis le tag git.
• Documenter/reproduire le build de perun.dll (CMake déjà présent).

5. DevOps / DBA / Release

Base de données — plutôt saine.

• InnoDB + utf8mb4/unicode_ci, PK/AUTO_INCREMENT, clés UNIQUE pertinentes (UCID, hash, type, stats par mission+ucid+type) et index sur les colonnes de tri (datetime, instance, type) — 04_MySQL/m1081_perun.sql.
• D2 🟢 : pas de FOREIGN KEY (intégrité référentielle non garantie), dépendance documentée à STRICT_TRANS_TABLES désactivé (README:42,84) — c'est-à-dire qu'on s'appuie sur la coercition/troncature silencieuse de MySQL, ce qui masque des bugs. Nom de fichier cryptique (m1081_perun.sql).
• Pas d'outil de migration (un seul dump). → introduire des migrations versionnées (Flyway/dbmate/sqitch) et, à terme, des FK + un mode strict assumé.

Build / release / repo

• D1 🟡 : arbre Lua 5.1.5 complet vendoré dans 03_Perun_Lua_Wrapper/lua-5.1.5/ → gonfle le repo et l'audit. Le passer en sous-module / téléchargement au build.
• Build manuel VS2017, pas de CI, pas d'artefacts reproductibles.
• Contributions historiquement attendues sur la branche dev (README:135).
• Pas de .gitignore racine (un seul dans le wrapper).

À faire

• CI (build C# + luacheck + lint PHP + validation du schéma SQL).
• Migrations DB versionnées ; activer les FK progressivement.
• Sortir Lua amont du repo ; pipeline de build de la DLL.
• Releases taguées avec binaires (perun.dll + app) attachés.

---

Feuille de route priorisée

P0 — Sécurité bloquante (avant toute prod)

1. Paramétrer 100 % des requêtes SQL (S1).
2. Bind loopback par défaut + auth/allowlist sur le listener TCP (S2).

P1 — Durcissement & conformité 3. Échappement HTML de l'exemple PHP (S3). 4. Chiffrer le mot de passe MySQL — DPAPI (S4). 5. Rétention/anonymisation IP & UCID, doc RGPD (S5). 6. File thread-safe bornée à la place du buffer tableau (B1).

P2 — Modernisation 7. Découper les monolithes, DTO typés au lieu de dynamic (A1). 8. Connexion poolée + accès DB async + ExecuteNonQuery (B2). 9. local + luacheck sur le hook, version unifiée depuis git (L1). 10. Cibler .NET 8 / worker multiplateforme. 11. Tests + CI.

P3 — Hygiène long terme 12. Migrations DB, FK, mode SQL strict assumé (D2). 13. Désvendoriser Lua, build reproductible de la DLL (D1). 14. Releases taguées + artefacts.

Plan de reprise suggéré

1. Geler le comportement : quelques tests de caractérisation sur le parsing de trames et la génération SQL, pour refactorer sans régresser.
2. Sprint sécu (P0) sur une branche security/sql-and-tcp, puis tag v0.13.0-rc testé en loopback.
3. P1 en incréments livrables.
4. Décider de la cible app (garder WinForms vs worker .NET 8) avant d'attaquer P2 — ça oriente tout le refactor.

Adapté à ton contexte : tu fais déjà tourner Gitea + CI sur le cluster ; un pipeline build/lint Perun s'y intègre directement, et la communauté Commus DCS est un terrain de test naturel pour les stats.