diff --git a/.gitignore b/.gitignore index 7419dd4..f308b32 100644 --- a/.gitignore +++ b/.gitignore @@ -1,27 +1,141 @@ -.env +# ============================================================================= +# Secrets et configuration locale +# ============================================================================= +# `.env*` couvre .env, .env.local, .env.production, .env.bak… +# L'exception laisse passer le seul fichier modèle, qui ne doit contenir +# que des valeurs factices. +.env* +!.env.example -.instance/ +*.key +*.pem +*.pfx +*.p12 +.certs/ +id_rsa +id_ed25519 +known_hosts +credentials.json +service-account*.json + +# ============================================================================= +# Assistants IA — Claude +# ============================================================================= +.claude/ +.claude.json +.claude/settings.local.json +CLAUDE.md +CLAUDE.local.md +.anthropic/ + +# ============================================================================= +# Assistants IA — ChatGPT / OpenAI +# ============================================================================= +.chatgpt/ +.openai/ +.codex/ +AGENTS.md +chatgpt-*.md +openai-*.md + +# ============================================================================= +# Assistants IA — autres outils +# ============================================================================= +.cursor/ +.cursorrules +.cursorignore +.windsurf/ +.windsurfrules +.aider* +.continue/ +.clinerules +.roo/ +.gemini/ +GEMINI.md +.github/copilot-instructions.md +.copilot/ + +# ============================================================================= +# Notes de travail et suivi générés par assistant IA +# ============================================================================= +# Rapports d'audit, plans, brouillons et notes de session : documents de +# travail, non destinés à être versionnés dans le dépôt applicatif. +.ai/ +audit/ +*.audit.md +NOTES-IA.md +TODO-IA.md + +# ============================================================================= +# Python +# ============================================================================= +__pycache__/ +*.py[cod] +*.so +*.egg-info/ +.eggs/ +build/ +dist/ + +# Environnements virtuels +venv/ +.venv/ +env/ +ENV/ + +# Outils +.pytest_cache/ +.ruff_cache/ +.mypy_cache/ +.coverage +.coverage.* +htmlcov/ +coverage_html_report/ +coverage.xml + +# ============================================================================= +# Données applicatives — ne jamais versionner +# ============================================================================= +# Base SQLite locale (une base de démo a déjà été committée par le passé, +# voir l'historique du commit `4fd27ba`). +instance/ *.db +*.sqlite +*.sqlite3 +# Contrats téléversés (données personnelles) documents/ -__pycache__/ -*.cpython-313.pyc -*.cpython-312.pyc -*.pyc +# Sauvegardes produites par app/supporting_scripts/backup.py +backups/ -.pytest_cache/ -.coverage - -htmlcov/ -.DS_Store +# Journaux applicatifs +logs/ *.log -.certs/ -*.pem +# État d'exécution du bot Discord +discord_pending.json -docs/ -*.html +# ============================================================================= +# Éditeurs et systèmes d'exploitation +# ============================================================================= +.idea/ +.vscode/ +*.swp +*.swo +.DS_Store +Thumbs.db +desktop.ini -instance/ -*.db \ No newline at end of file +# ============================================================================= +# NOTE — règles retirées volontairement +# ============================================================================= +# `docs/` et `*.html` figuraient ici et ignoraient TOUT fichier .html du +# dépôt, y compris les templates Jinja2 de app/templates/. Un nouveau +# template était donc invisible pour git : l'application fonctionnait en +# local et cassait en production avec une TemplateNotFound, sans que +# `git status` ne signale quoi que ce soit. +# +# Les rapports de couverture HTML, qui étaient vraisemblablement la cible +# de ces règles, sont couverts ci-dessus par `htmlcov/` et +# `coverage_html_report/`. diff --git a/audit/00-provenance.md b/audit/00-provenance.md deleted file mode 100644 index bd823ff..0000000 --- a/audit/00-provenance.md +++ /dev/null @@ -1,149 +0,0 @@ -# 0 — Provenance et écart entre les dépôts - -## Deux dépôts, un seul fait autorité - -| Dépôt | Rôle | HEAD `main` | -|---|---|---| -| `git.immortal.host/clubesportsudes/team-tryouts` | **Référence** — c'est celui-ci qui est audité | `bb0bc1c` | -| `github.com/cedrick2711/team-tryouts` | Miroir, **en retard de 16 commits** | `08f02f7` | - -Les deux partagent un ancêtre commun (`d6fe505`). Le miroir GitHub porte en outre 2 commits qui ne sont pas sur le dépôt de référence (`0d5a408 fix registration` et son merge `08f02f7`) ; le correctif correspondant existe sur `immortal` sous la forme du commit `7d30aff Corriger erreur d'enregistrement`. - -**La première passe de cet audit a été menée sur le miroir GitHub**, avant que la bonne source ne soit connue. L'ensemble des constats a été revérifié contre `immortal/main`. Le présent document liste ce qui a changé. - -### Les 16 commits d'écart - -``` -bb0bc1c ajout de plateforme de base pour les url TRN -f89e4de régler problème avec mise a jour des status de message du discord bot -fcf10bc bug fix: Manager ne pouvait pas voir les tryouts. probleme avec discord bot -68e3da6 fix probleme avec dispos -d25b35c régler erreur 500 sur changement de role par admin -aeba4d7 régler problème de changement de rôle -7bb8022 added a clear_db to start fresh with only an admin -0dd4ecd Erreur de frappe dans un des dossiers -0f7788e Changement du layout de la page d'enregistrement -47d5ec4 added discord oauth2 to get basic user info to complete profile when registering -10af8c0 small change for prod -7d30aff Corriger erreur d'enregistrement -4097416 Update wsgi.py -056cea0 Update .gitea/workflows/git-to-ptero.yaml -d3b5700 Add .gitea/workflows/git-to-ptero.yaml -fd258de Update app/.env.exemple -``` - ---- - -## Constats de la première passe devenus caducs - -Quatre constats ont été résolus par l'équipe entre les deux points de l'historique. Ils sont conservés ici pour mémoire, et retirés du décompte des documents 01 à 03. - -### ✅ Seed automatique en production — résolu - -Le bloc de `app/app.py` qui déclenchait le seed quand la table `users` était vide a été supprimé (commit `7bb8022`), et `app/supporting_scrits/seed.py` (460 lignes, comptes `manager1`, `coach1`, `scout1`… tous avec le mot de passe `password`) a été supprimé. - -```diff - with app.app_context(): - import app.models as models -- from app.models import User - db.create_all() -- -- # Seed database if empty -- if User.query.count() == 0: -- from app.supporting_scrits.seed import seed_database -- seed_database() -``` - -Un déploiement neuf ne crée donc plus de comptes par défaut. - -> **Reste à traiter.** Le script de remplacement `clear_db.py` code toujours en dur `admin` / `password`, et il est bien plus dangereux que l'ancien seed sur un autre plan. Voir [SEC-22](01-securite.md#sec-22--). -> -> **À vérifier en production**, indépendamment du code : les comptes créés par l'ancien seed (`admin`, `manager1`, `manager2`, `coach1`, `coach2`, `coach3`, `scout1`) peuvent toujours exister en base avec le mot de passe `password`. La suppression du script ne supprime pas les comptes qu'il a créés. - -### ✅ Token Discord imprimé sur stdout — résolu - -La ligne `print(DISCORD_BOT_TOKEN or 'FAILED TO PRINT BOT TOKEN')` de `app/discord_bot.py:25` a été supprimée. Le module ne comporte plus aucun `print()`. - -### ✅ `proxy_pass` vers `0.0.0.0` — résolu - -`app/nginx.conf:122` : `proxy_pass http://0.0.0.0:5000;` → `proxy_pass http://127.0.0.1:5000;` - -> **Reste à traiter.** L'incohérence de ports subsiste : `wsgi.py:24` utilise toujours `PORT` par défaut à **10000** alors que nginx envoie vers **5000** et que le docstring du même fichier annonce 5000. Voir [STD-06](03-standards-stack.md#std-06--). - -### ✅ Faute de frappe `supporting_scrits` — résolu - -Le dossier a été renommé `app/supporting_scripts/` (commit `0dd4ecd`). - -> **Effet de bord non traité.** Le job CI « Security Scan » invoquait déjà un chemin erroné (`python security_scan.py` à la racine) ; le renommage l'éloigne encore. Voir [MNT-04](02-maintenabilite.md#mnt-04--). - ---- - -## Constats aggravés ou requalifiés - -### ⚠️ Secrets exposés — le nettoyage n'a pas révoqué - -`app/.env.exemple` a été nettoyé (commit `fd258de`) : les valeurs réelles ont été remplacées par des marqueurs (`flask_app_secret_key`, `my_discord_bot_token`, `URI_vers_db_posgres`). - -**Mais les secrets restent intégralement récupérables.** Vérification par recherche dans l'historique complet des deux dépôts : - -``` -$ git log --all --oneline -S "MTUyNzY3ODU3NjUyNTA1NDEyNQ" -fd258de Update app/.env.exemple ← retrait -2d3721b Ajout d'un .env.exemple pour simplifier la collaboration ← introduction -``` - -Le blob contenant le token Discord, le mot de passe PostgreSQL et la `SECRET_KEY` est toujours atteignable par `git show 2d3721b:app/.env.exemple` sur **les deux dépôts**. Il est de surcroît **toujours dans le HEAD du miroir GitHub**, donc visible par simple navigation dans l'interface web. - -Le constat reste donc critique et sa correction inchangée : **révoquer, puis purger**. Voir [SEC-01](01-securite.md#sec-01--). - -### ⚠️ En-têtes de proxy — le correctif a inversé le risque - -`wsgi.py` a reçu un bloc de configuration proxy (commit `4097416`) : - -```python -trusted_proxy='*', -trusted_proxy_count=1, -trusted_proxy_headers={'x-forwarded-for', 'x-forwarded-proto'}, -clear_untrusted_proxy_headers=True -``` - -L'intention est bonne, et `clear_untrusted_proxy_headers=True` est le bon réflexe. Mais `trusted_proxy='*'` signifie « faire confiance aux en-têtes de proxy quelle qu'en soit la provenance », et `HOST` vaut toujours `0.0.0.0` par défaut (`wsgi.py:26`). - -Avant ce changement, `request.remote_addr` valait toujours l'IP de nginx : le rate limiting était appliqué à un seau global — gênant, mais fermé. Désormais, un attaquant qui atteint directement le port applicatif contrôle `X-Forwarded-For` et peut donc **présenter une IP différente à chaque requête**, ce qui neutralise le rate limiting et le suivi de tentatives par IP. - -Le constat change de nature et reste élevé. Voir [SEC-05](01-securite.md#sec-05--). - -### ⚠️ Identifiant Discord — la vérification ajoutée n'en est pas une - -Un flux OAuth2 Discord complet a été ajouté à l'inscription (commit `47d5ec4`, `auth.py:326-456`). Il obtient de Discord un identifiant authentifié et le place en session. - -Mais cet identifiant est ensuite réinjecté dans le formulaire d'inscription **par un champ caché** : - -```html - - -``` - -et c'est cette valeur — passée par le client — qui est enregistrée (`auth.py:253`, `:290`). Un utilisateur peut la modifier avant envoi, ou poster directement le formulaire sans jamais passer par Discord. - -`RegisterSchema` valide désormais le format de `discord_user_id` (17-20 chiffres, `validators.py:211-215`), ce qui est un progrès, mais ne prouve rien sur la propriété du compte. - -La conséquence pratique est inchangée par rapport à la première passe — et le risque de fausse assurance est nouveau, puisque le flux *paraît* vérifié. Voir [SEC-07](01-securite.md#sec-07--). - -### ⚠️ Limite d'inscription desserrée - -`auth.py:190` : `@limiter.limit("3 per hour")` → `@limiter.limit("20 per hour")`. - -Le CAPTCHA arithmétique étant trivial ([SEC-13](01-securite.md#sec-13--)), le rate limiting était la protection réellement efficace contre la création automatisée de comptes. Passer à 20/heure la divise par un facteur ~7, et [SEC-05](01-securite.md#sec-05--) la rend contournable. - ---- - -## Éléments nouveaux ajoutés à l'audit - -| Élément | Constat | -|---|---| -| `auth.py:326-456` — flux OAuth2 Discord | [SEC-20](01-securite.md#sec-20--) absence de paramètre `state` · [SEC-07](01-securite.md#sec-07--) identité non contraignante | -| `.gitea/workflows/git-to-ptero.yaml` | [SEC-21](01-securite.md#sec-21--) déploiement de `.git/` par SFTP | -| `clear_db.py` | [SEC-22](01-securite.md#sec-22--) script destructif sans garde-fou | -| `migrations/add_tryout_coaches.py` | [STD-01](03-standards-stack.md#std-01--) migration manuelle hors outillage | -| `discord_pending.json` | [MNT-16](02-maintenabilite.md#mnt-16--) état d'exécution versionné | diff --git a/audit/01-securite.md b/audit/01-securite.md deleted file mode 100644 index 2c7f272..0000000 --- a/audit/01-securite.md +++ /dev/null @@ -1,809 +0,0 @@ -# 1 — Sécurité - -22 constats. Références de lignes sur `immortal/main` @ `bb0bc1c`. - -Les constats résolus par l'équipe (ancien SEC-02 seed automatique, ancien SEC-04 token imprimé) sont archivés dans [`00-provenance.md`](00-provenance.md). Les identifiants des constats subsistants n'ont pas été renumérotés, pour préserver la traçabilité. - -| ID | Constat | Sévérité | -|---|---|---| -| [SEC-01](#sec-01--) | Secrets de production toujours récupérables dans l'historique | 🔴 Critique | -| [SEC-03](#sec-03--) | CORS ouvert à toutes les origines avec credentials par défaut | 🔴 Critique | -| [SEC-05](#sec-05--) | `trusted_proxy='*'` sur interface publique → en-têtes de proxy usurpables | 🟠 Élevé | -| [SEC-06](#sec-06--) | Schémas de validation importés mais jamais appliqués sur les routes utilisateurs | 🟠 Élevé | -| [SEC-07](#sec-07--) | Identifiant Discord modifiable malgré l'OAuth2 → détournement des notifications | 🟠 Élevé | -| [SEC-08](#sec-08--) | Rate limiting en mémoire, non partagé et réinitialisé à chaque redémarrage | 🟠 Élevé | -| [SEC-20](#sec-20--) | Flux OAuth2 Discord sans paramètre `state` | 🟠 Élevé | -| [SEC-21](#sec-21--) | Le déploiement SFTP pousse le répertoire `.git/` sur le serveur | 🟠 Élevé | -| [SEC-09](#sec-09--) | `nl2br` marque du HTML utilisateur non échappé comme sûr | 🟡 Moyen | -| [SEC-10](#sec-10--) | CSP avec `'unsafe-inline'` sur `script-src` | 🟡 Moyen | -| [SEC-11](#sec-11--) | Aucune validation de type sur l'upload de contrat signé | 🟡 Moyen | -| [SEC-12](#sec-12--) | Énumération d'utilisateurs via les messages de login | 🟡 Moyen | -| [SEC-13](#sec-13--) | CAPTCHA trivial, et limite d'inscription desserrée à 20/heure | 🟡 Moyen | -| [SEC-14](#sec-14--) | `/health` expose l'erreur brute de la base de données | 🟡 Moyen | -| [SEC-15](#sec-15--) | Profil complet de tout utilisateur visible par tout compte authentifié | 🟡 Moyen | -| [SEC-22](#sec-22--) | `clear_db.py` : destruction totale sans garde-fou, identifiants en dur | 🟡 Moyen | -| [SEC-16](#sec-16--) | Conversions `int()` non protégées sur entrées utilisateur | 🔵 Faible | -| [SEC-17](#sec-17--) | `add_to_team` ne vérifie pas la cohérence tryout/équipe/joueur | 🔵 Faible | -| [SEC-18](#sec-18--) | Aucune réinitialisation de mot de passe ni MFA | 🔵 Faible | -| [SEC-19](#sec-19--) | Journal d'audit d'authentification déclaré mais jamais alimenté | 🔵 Faible | - ---- - -## SEC-01 · 🔴 - -**Secrets de production toujours récupérables dans l'historique** - -Le fichier `app/.env.exemple` a été nettoyé au commit `fd258de` : les valeurs réelles y sont désormais remplacées par des marqueurs. **Cela ne change rien au fait que les secrets sont compromis.** - -Recherche sur l'ensemble des références des deux dépôts : - -``` -$ git log --all --oneline -S "MTUyNzY3ODU3NjUyNTA1NDEyNQ" -fd258de Update app/.env.exemple -2d3721b Ajout d'un .env.exemple pour simplifier la collaboration - -$ git log --all --oneline -S "0YO038Od2QcQCsCTlNDAMOAOlcPGrIND" -fd258de Update app/.env.exemple -2d3721b Ajout d'un .env.exemple pour simplifier la collaboration -``` - -Le blob introduit en `2d3721b` reste atteignable par `git show 2d3721b:app/.env.exemple` sur **`immortal` comme sur GitHub**. Il contient : - -| Secret | Portée | -|---|---| -| `DISCORD_BOT_TOKEN` | Token du bot UdeS Esports — le commentaire du fichier le confirmait explicitement | -| `DATABASE_URL` | PostgreSQL Render, hôte public, **avec mot de passe** | -| `SECRET_KEY` | Clé de signature des sessions Flask | - -Aggravant : **le miroir GitHub porte encore ces valeurs dans son HEAD**. Elles sont visibles par simple navigation dans l'interface web, sans même cloner. - -**Impact.** Inchangé depuis la première passe : -- accès lecture/écriture complet à la base de production — identités, courriels, téléphones, notes personnelles, contrats ; -- contrôle du bot Discord, donc envoi de messages privés en usurpant l'identité de l'organisation ; -- avec la `SECRET_KEY`, **forge de cookies de session Flask valides** — authentification en tant que n'importe quel utilisateur, y compris `admin`, sans mot de passe. - -**Correction.** Dans cet ordre : - -1. **Révoquer**, avant toute manipulation de git — c'est la seule action qui rend les valeurs divulguées inoffensives : - - régénérer le token du bot (Discord Developer Portal → Bot → Reset Token) ; - - faire tourner le mot de passe PostgreSQL sur Render ; - - régénérer `SECRET_KEY` (`python -c "import secrets; print(secrets.token_hex(32))"`) — cela invalidera toutes les sessions en cours, ce qui est le comportement souhaité. -2. **Purger l'historique des deux dépôts** : - ```bash - git filter-repo --path app/.env.exemple --invert-paths - ``` - puis forcer la réécriture sur toutes les branches, et prévenir les collaborateurs qu'ils doivent recloner. -3. **Traiter le miroir GitHub** : soit le synchroniser après purge, soit le supprimer. Un miroir en retard de 16 commits qui expose des secrets dans son HEAD n'apporte rien et coûte beaucoup. -4. Ajouter `.env*` au `.gitignore` avec une exception explicite : `!.env.example`. -5. Ajouter un scan de secrets à la CI (`gitleaks`, ou `detect-secrets` en pre-commit). - -> Renommer aussi le fichier en `.env.example` — `exemple` est un francisme qui échappe à la détection de la plupart des outils. - ---- - -## SEC-03 · 🔴 - -**CORS ouvert à toutes les origines avec credentials par défaut** - -`app/app.py:76-95` — inchangé depuis la première passe. - -```python -allowed_origins = os.getenv('CORS_ALLOWED_ORIGINS', '').split(',') -allowed_origins = [origin.strip() for origin in allowed_origins if origin.strip()] - -if allowed_origins: - CORS(app, origins=allowed_origins, supports_credentials=True, …) -else: - # When no origins specified, allow all (development) or none (production) - # In production with a reverse proxy, CORS is handled at the Nginx level - CORS(app, supports_credentials=True, methods=[…], max_age=3600) -``` - -Le commentaire décrit une intention non implémentée : la branche `else` **autorise toutes les origines**, en développement comme en production. `flask-cors` avec `supports_credentials=True` et sans `origins` reflète l'en-tête `Origin` de la requête dans `Access-Control-Allow-Origin` et ajoute `Access-Control-Allow-Credentials: true`. - -Le commentaire renvoie la responsabilité à nginx, mais `app/nginx.conf` **ne contient aucune directive CORS**. Et `app/.env.exemple` — y compris dans sa version nettoyée — **ne définit toujours pas `CORS_ALLOWED_ORIGINS`** : la configuration livrée aux équipes tombe donc systématiquement dans la branche permissive. - -**Impact.** N'importe quel site tiers visité par un utilisateur connecté peut lire, avec ses cookies de session, le contenu de toutes les routes `GET` : -- `/users/disponibilities` — disponibilités de tous les joueurs actifs, avec noms d'utilisateur ; -- `/matches/api/events` — calendrier complet, participants, sessions 1:1 approuvées ; -- `/users/profile`, `/users//view` — données personnelles. - -La protection CSRF limite les écritures, mais n'empêche pas ces lectures. - -**Correction.** - -```python -allowed_origins = [o.strip() for o in os.getenv('CORS_ALLOWED_ORIGINS', '').split(',') if o.strip()] -if allowed_origins: - CORS(app, origins=allowed_origins, supports_credentials=True, methods=[...], max_age=3600) -elif app.debug: - CORS(app, origins=['http://localhost:5000'], supports_credentials=True) -# sinon : pas de CORS du tout — même origine uniquement -``` - -L'application étant rendue côté serveur et consommant ses propres API en même-origine, **le cas nominal est de ne pas activer CORS du tout**. Documenter `CORS_ALLOWED_ORIGINS` dans le fichier d'exemple. - ---- - -## SEC-05 · 🟠 - -**`trusted_proxy='*'` sur interface publique → en-têtes de proxy usurpables** - -`wsgi.py:29-42` - -```python -host = os.getenv('HOST', '0.0.0.0') # Bind to localhost by default (Nginx reverse proxy) -... -serve( - app, host=host, port=port, threads=threads, - channel_timeout=30, cleanup_interval=30, - # Waitress Proxy Settings - trusted_proxy='*', - trusted_proxy_count=1, - trusted_proxy_headers={'x-forwarded-for', 'x-forwarded-proto'}, - clear_untrusted_proxy_headers=True -) -``` - -L'intention est correcte et `clear_untrusted_proxy_headers=True` est le bon réflexe. Deux réglages annulent le bénéfice : - -- **`trusted_proxy='*'`** demande à Waitress d'accepter `X-Forwarded-For` et `X-Forwarded-Proto` **de n'importe quel pair**, pas seulement de nginx ; -- **`HOST` vaut `0.0.0.0`** — le commentaire de la ligne 26 annonce l'inverse de ce que fait le code. Le port applicatif est donc joignable directement, en contournant nginx. - -**Impact.** La combinaison des deux rend les en-têtes de proxy entièrement contrôlables par un attaquant qui atteint le port applicatif : - -1. **Contournement du rate limiting et du suivi de tentatives.** `app/extensions.py:16-19` utilise `key_func=get_remote_address`, qui lit `request.remote_addr` — valeur que Waitress renseigne désormais depuis `X-Forwarded-For`. Un attaquant qui change cet en-tête à chaque requête obtient **un seau de rate limiting neuf à chaque fois** : la limite de `10 per minute` sur `/auth/login` (`auth.py:96`) ne s'applique plus, ni les limites par défaut `200/jour, 50/heure`. - - Il faut noter que ce changement **a inversé la nature du risque**. Avant, `remote_addr` valait toujours l'IP de nginx : la protection était mal calibrée (un seau global partagé par tous) mais fermée. Elle est maintenant ouverte. - - Le verrouillage de compte après 5 échecs (`auth.py:167-181`) reste opérant, puisqu'il est indexé sur l'utilisateur et non sur l'IP — c'est aujourd'hui la seule barrière anti-bruteforce réellement en place. - -2. **Contournement de la redirection HTTPS.** `app/app.py:182` teste `X-Forwarded-Proto`. En l'envoyant à `https` sur une connexion en clair, on désactive `force_https()` et l'application se comporte comme si la connexion était sécurisée. Même effet sur l'émission de HSTS (`app.py:163`). - -3. **Journalisation empoisonnée.** Toute IP écrite dans les logs devient une valeur fournie par le client — ce qui rend l'analyse post-incident peu fiable (et affaiblit d'avance la correction proposée en [SEC-19](#sec-19--)). - -**Correction.** - -```python -host = os.getenv('HOST', '127.0.0.1') -... - trusted_proxy=os.getenv('TRUSTED_PROXY', '127.0.0.1'), - trusted_proxy_count=1, - trusted_proxy_headers={'x-forwarded-for', 'x-forwarded-proto'}, - clear_untrusted_proxy_headers=True, -``` - -Les deux corrections sont nécessaires : `trusted_proxy` restreint *qui* peut envoyer ces en-têtes, `HOST` restreint *qui peut se connecter*. Si l'hébergement Pterodactyl impose une écoute sur `0.0.0.0`, renseigner alors l'adresse réelle du proxy dans `TRUSTED_PROXY` et filtrer le port au pare-feu. - -Vérifier ensuite que `request.remote_addr` renvoie bien l'IP cliente, et non celle du proxy, avant de considérer le rate limiting comme fonctionnel. - ---- - -## SEC-06 · 🟠 - -**Schémas de validation importés mais jamais appliqués sur les routes utilisateurs** - -`app/routes/users.py:23-26` importe `CreateUserSchema`, `EditUserSchema` et `EditProfileSchema`. Chacun de ces trois noms n'apparaît **qu'une seule fois dans le fichier — sur la ligne d'import**. Ils ne sont jamais instanciés. - -Les trois routes concernées lisent le formulaire brut : - -| Route | Lignes | Traitement du mot de passe | -|---|---|---| -| `edit_user` | `:88-152` | `request.form.get('password')` → `hash_password()` si non vide | -| `create_user` | `:201-241` | `request.form.get('password')` → `hash_password()` sans contrôle | -| `edit_profile` | `:274-320` | idem, sur son propre compte | - -Comparaison avec `auth.py:228-230`, où `RegisterSchema` **est** correctement chargé — l'inscription publique est validée, mais pas les trois chemins de création/modification de compte. - -**Impact.** -- **Aucune politique de mot de passe** sur ces routes : `a` est accepté. Un président créant les comptes de l'équipe peut leur attribuer des mots de passe d'un caractère, et n'importe quel utilisateur peut affaiblir le sien via `edit_profile`. -- **Aucune validation de format** sur `email` (le champ n'est même pas vérifié comme étant une adresse), `username`, `phone`. -- `create_user` appelle `hash_password(password)` sans vérifier que `password` est non vide : le hachage d'une chaîne vide est stocké, et le compte devient accessible avec un mot de passe vide. -- Dans `edit_user`, `full_name` et `email` sont assignés sans contrôle de nullité alors que les colonnes sont `nullable=False` (`models/user_model/user.py:20-21`) → `IntegrityError` non gérée → 500. - -**Correction.** Appliquer les schémas déjà écrits, sur le modèle de `auth.py` : - -```python -schema = EditProfileSchema() -try: - validated = schema.load(request.form) -except ValidationError as err: - for field, messages in err.messages.items(): - for msg in messages: - flash(f'{field}: {msg}', 'danger') - return render_template('pages/edit_profile.html', ...) -``` - -Attention : `EditUserSchema` et `EditProfileSchema` déclarent `password` avec `load_default=''` et `validate=validate_password` — un mot de passe vide échouera donc la validation. Retirer le champ du payload quand il est vide avant le `load()`. - -Point d'attention pour `edit_user` : le bloc de changement de rôle par SQL brut (`:114-126`) exécute `db.session.remove()`. Toute validation doit intervenir **avant** ce bloc, sinon l'objet `user` manipulé ensuite n'est plus celui qui a été validé. - ---- - -## SEC-07 · 🟠 - -**Identifiant Discord modifiable malgré l'OAuth2 → détournement des notifications** - -Un flux OAuth2 Discord complet a été ajouté (`auth.py:326-456`). Il obtient de Discord un identifiant authentifié et le stocke en session : - -```python -session['discord_oauth'] = { - 'id': user_data.get('id'), # auth.py:448 — valeur authentifiée par Discord - 'username': user_data.get('username'), - ... -} -``` - -Mais cette valeur est ensuite **réinjectée dans le formulaire par un champ caché** : - -```html - - -``` - -et c'est la valeur **renvoyée par le client** qui est enregistrée (`auth.py:253` puis `:290`) : - -```python -discord_user_id = validated.get('discord_user_id') -... -user = Player(..., discord_user_id=discord_user_id, ...) -``` - -**La preuve d'identité obtenue de Discord est perdue au passage.** Un utilisateur peut modifier le champ caché avant envoi, ou poster directement le formulaire sans jamais avoir ouvert le flux OAuth2. `RegisterSchema` valide désormais le format (`validators.py:211-215` : 17-20 chiffres), ce qui est un progrès, mais ne prouve rien sur la propriété du compte. - -La route `edit_profile` (`users.py:284`, `:305`), accessible à **tout utilisateur authentifié**, permet de la même façon de définir un identifiant arbitraire — et n'applique même pas la validation de format ([SEC-06](#sec-06--)). Aucune contrainte d'unicité n'existe sur la colonne (`models/user_model/user.py:32`). - -**Impact.** Un joueur peut renseigner l'identifiant Discord d'une autre personne — un coach, un membre de la direction. Il reçoit alors à sa place les messages privés du bot : -- demandes de sessions 1:1 avec leurs *« discussion points »*, souvent confidentiels ; -- notifications de matchs et d'entraînements ; -- surtout, **la capacité de répondre à la place de la cible** : `discord_bot.py` traite les réactions ✅/❌ en message privé pour accepter ou refuser une demande 1:1. L'attaquant obtient un pouvoir de décision qui ne lui appartient pas. - -Deux utilisateurs peuvent en outre déclarer le même identifiant, ce qui rend le comportement non déterministe. - -**Risque supplémentaire, nouveau :** le flux *paraît* vérifié. Un relecteur qui constate la présence d'un OAuth2 Discord conclura raisonnablement que l'identifiant est authentifié. Il ne l'est pas. - -**Correction.** La brique manquante est courte — la valeur authentifiée existe déjà en session, il suffit de ne plus la faire transiter par le client : - -```python -# auth.py, dans register() — ignorer ce que le formulaire prétend -discord_oauth = session.get('discord_oauth') or {} -discord_user_id = discord_oauth.get('id') # seule source de vérité -discord_username = discord_oauth.get('username') -``` - -et retirer le champ caché de `register.html:61` (l'afficher en lecture seule si l'on veut le montrer à l'utilisateur). - -En complément : -1. Ajouter une contrainte d'unicité sur `User.discord_user_id` (nécessite [STD-01](03-standards-stack.md#std-01--)). -2. Pour `edit_profile`, soit passer par le même flux OAuth2, soit réserver la modification de ce champ aux administrateurs. -3. Traiter [SEC-20](#sec-20--) : sans paramètre `state`, le contenu de `session['discord_oauth']` n'est lui-même pas digne de confiance. - ---- - -## SEC-08 · 🟠 - -**Rate limiting en mémoire, non partagé et réinitialisé à chaque redémarrage** - -`app/extensions.py:16-19` - -```python -limiter = Limiter( - key_func=get_remote_address, - default_limits=["200 per day", "50 per hour"] -) -``` - -Aucun `storage_uri` n'est fourni. Flask-Limiter bascule sur son backend `memory://`, explicitement documenté comme non destiné à la production. - -**Impact.** -- L'état est **par processus**. Toute évolution vers plusieurs workers ou plusieurs instances fragmente les compteurs et multiplie d'autant la limite effective. -- L'état est **perdu à chaque redémarrage** : les compteurs se réinitialisent à chaque redéploiement. -- Combiné à [SEC-05](#sec-05--), la protection est de toute façon indexée sur une clé que le client contrôle. - -**Correction.** Adosser le limiteur à un stockage partagé — Redis de préférence, ou la base PostgreSQL déjà présente : - -```python -limiter = Limiter( - key_func=get_remote_address, - default_limits=["200 per day", "50 per hour"], - storage_uri=os.getenv('RATELIMIT_STORAGE_URI', 'memory://'), -) -``` - -Réévaluer aussi les valeurs : `50 per hour` par IP est très bas pour une application rendue côté serveur, où chaque page consomme plusieurs requêtes (`/matches/api/events`, `/users/disponibilities`…). Exclure `/static` et `/health` du décompte. - -Corriger [SEC-05](#sec-05--) d'abord : sans clé fiable, un stockage partagé ne sert à rien. - ---- - -## SEC-20 · 🟠 - -**Flux OAuth2 Discord sans paramètre `state`** - -`app/routes/auth.py:340-348` - -```python -params = { - 'client_id': DISCORD_CLIENT_ID, - 'redirect_uri': DISCORD_REDIRECT_URI, - 'response_type': 'code', - 'scope': 'identify connections', -} -query = '&'.join(f'{k}={requests.utils.quote(v)}' for k, v in params.items()) -auth_url = f'{DISCORD_API_BASE}/oauth2/authorize?{query}' -``` - -Aucun paramètre `state` n'est émis, et `discord_callback` (`:351-456`) n'en vérifie aucun : la requête de retour est acceptée sur la seule présence d'un `code` (`:363-366`). - -`state` est le mécanisme anti-CSRF prévu par la RFC 6749 (§10.12) : une valeur aléatoire liée à la session, émise à l'aller et vérifiée au retour. - -**Impact.** Un attaquant initie le flux avec **son propre** compte Discord, intercepte le `code` d'autorisation sans le consommer, puis amène la victime à visiter : - -``` -https:///auth/discord/callback?code= -``` - -Le serveur échange ce code, obtient le profil Discord **de l'attaquant**, et le place dans `session['discord_oauth']` — la session **de la victime**. Celle-ci voit alors le formulaire d'inscription pré-rempli avec l'identité Discord de l'attaquant, et l'enregistre sans raison de se méfier (le bandeau affiche *« Discord account connected! »*, `auth.py:455`). - -Combiné à [SEC-07](#sec-07--), c'est un chemin direct pour faire enregistrer à un tiers un `discord_user_id` contrôlé par l'attaquant — donc pour recevoir les notifications privées de ce compte. - -Le flux ne servant aujourd'hui qu'à pré-remplir un formulaire d'inscription, la portée est bornée. Elle s'étendrait immédiatement si l'OAuth2 était réutilisé pour l'authentification ou pour la liaison de compte a posteriori, ce qui est l'évolution naturelle de ce code. - -**Correction.** - -```python -# auth.py — discord_login() -state = secrets.token_urlsafe(32) -session['discord_oauth_state'] = state -params = {..., 'state': state} - -# auth.py — discord_callback() -expected = session.pop('discord_oauth_state', None) -if not expected or not secrets.compare_digest(request.args.get('state', ''), expected): - flash('Requête Discord invalide.', 'danger') - return redirect(url_for('auth.register')) -``` - -Deux points secondaires dans le même bloc : -- `requests.utils.quote(v)` (`:346`) lève un `TypeError` si `DISCORD_REDIRECT_URI` n'est pas défini — seul `DISCORD_CLIENT_ID` est vérifié (`:336`). Vérifier les trois variables, ou construire l'URL avec `urllib.parse.urlencode`. -- `:389` — `flash(f'Failed to connect to Discord. Please try again.', 'danger')` est une f-string sans substitution, et la variable `e` capturée à la ligne 388 n'est pas utilisée. Sans conséquence, mais signalé par Ruff (`F541`, `F841`). - ---- - -## SEC-21 · 🟠 - -**Le déploiement SFTP pousse le répertoire `.git/` sur le serveur** - -`.gitea/workflows/git-to-ptero.yaml:41` - -```yaml -mirror -R --verbose --parallel=4 ./ ./ -``` - -`mirror -R` téléverse le répertoire courant du runner vers la racine distante. Après un `actions/checkout`, ce répertoire contient **`.git/` en entier** — objets, packfiles, refs, historique complet. - -**Impact.** Si la racine du dépôt est aussi la racine servie par le serveur web — ce qui est le cas par défaut sur un hébergement de type Pterodactyl où l'application est déployée telle quelle — alors `.git/` devient téléchargeable. Les outils d'exploitation courants (`git-dumper` et équivalents) reconstruisent l'intégralité du dépôt à partir de `.git/config`, `.git/HEAD` et des objets, même sans listing de répertoire activé. - -L'attaquant obtient alors le code source complet **et tout l'historique** — donc le commit `2d3721b` et les secrets de production de [SEC-01](#sec-01--). Les deux constats se composent : la purge d'historique demandée en SEC-01 perd son intérêt si le déploiement republie l'historique à chaque exécution. - -Deux points secondaires du même workflow : - -- **`StrictHostKeyChecking=no`** (`:35`) désactive la vérification de l'empreinte du serveur SFTP : le premier échange est vulnérable à une interception, et la clé privée de déploiement peut être présentée à un serveur usurpateur. Utiliser un `known_hosts` épinglé, alimenté par `ssh-keyscan` une fois et stocké en secret de dépôt. -- **Aucune exclusion.** Outre `.git/`, le miroir pousse `__pycache__/`, les éventuels `logs/` et tout fichier résiduel du runner. - -**Correction.** - -```yaml -mirror -R --verbose --parallel=4 \ - --exclude-glob .git/ \ - --exclude-glob .github/ \ - --exclude-glob .gitea/ \ - --exclude-glob __pycache__/ \ - --exclude-glob '*.pyc' \ - --exclude-glob logs/ \ - --exclude-glob documents/ \ - ./ ./ -``` - -Plus robuste : construire un artefact de déploiement propre (`git archive HEAD | tar -x -C dist/`) et ne téléverser que `dist/`. `git archive` n'inclut par construction ni `.git/` ni les fichiers ignorés. - -Vérifier par ailleurs, sur le serveur actuel, si `.git/` est déjà présent et accessible — le workflow est déclenchable manuellement (`workflow_dispatch`, `:4`) et a pu être exécuté. Le cas échéant, supprimer le répertoire distant en plus d'appliquer le correctif. - ---- - -## SEC-09 · 🟡 - -**`nl2br` marque du HTML utilisateur non échappé comme sûr** - -`app/app.py:19-30` - -```python -def nl2br(value): - if value: - return markupsafe.Markup('
'.join(str(value).splitlines())) - return '' -``` - -`markupsafe.Markup()` **désactive l'échappement automatique de Jinja2** pour la chaîne produite. Le contenu utilisateur est inséré tel quel, sans passer par `escape()`. - -**Statut actuel : non exploitable.** Aucune utilisation de `|nl2br` dans les templates. Le filtre est enregistré (`app.py:125`) mais mort. - -**Risque.** Piège en attente : le filtre porte un nom naturel, il est enregistré globalement, et la première personne qui écrira `{{ note.content|nl2br }}` — usage évident sur `PersonalNote`, `TeamNote` ou les *discussion points* des 1:1 — introduira une XSS stockée sans s'en rendre compte. Ces contenus sont saisis par des coachs et joueurs, et affichés à d'autres utilisateurs. - -**Correction.** Échapper avant de marquer : - -```python -from markupsafe import Markup, escape - -def nl2br(value): - if not value: - return '' - return Markup('
').join(escape(str(value)).splitlines()) -``` - -Alternative sans code : supprimer le filtre et utiliser `white-space: pre-line` en CSS. - ---- - -## SEC-10 · 🟡 - -**CSP avec `'unsafe-inline'` sur `script-src`** - -`app/app.py:149-159` - -```python -"script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; " -"style-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com https://cdn.jsdelivr.net; " -"img-src 'self' data: https://cdn.discordapp.com; " -``` - -`'unsafe-inline'` sur `script-src` **annule l'essentiel du bénéfice de la CSP** : c'est précisément l'injection de `