AUDIT-2026-07-29.md
Code Preview
# Audit de sécurité — nouvelles surfaces d'attaque (2026-07-29)
- **Périmètre** : uniquement ce qui a été ajouté depuis le premier audit
(`AUDIT.md`, commit `523f049`) — clone HTTPS anonyme, notifications
fédérées, dépôts épinglés, historique de commits, croissance du RPC
admin, et le changement d'Ingress qui les accompagne.
- **Nature** : revue de code adversariale + tests de reproduction pour
chaque correctif.
- **État** : les correctifs identifiés ci-dessous sont **déjà appliqués**
dans ce commit, avec un test de régression pour chacun des deux qui s'y
prêtaient (`internal/store/notifications_test.go`,
`internal/federation/notify_test.go`).
---
## 1. Synthèse
Aucune faille critique (pas de fuite de dépôt privé, pas de contournement
de l'authentification, pas d'injection). Les points trouvés sont des trous
de limitation de débit / de plafonnement — la même catégorie que H1 dans
l'audit précédent, désormais étendue aux deux nouvelles surfaces qui
acceptent du trafic non authentifié : le clone HTTP anonyme et la
réception de notifications fédérées.
| ID | Sévérité | Titre | Statut |
|----|----------|-------|--------|
| M1 | 🟠 Medium | Aucune limitation de débit sur le clone HTTP anonyme | ✅ corrigé |
| M2 | 🟠 Medium | Notifications non plafonnées par destinataire | ✅ corrigé |
| M3 | 🟠 Medium | Destinataire de notification jamais vérifié comme utilisateur réel | ✅ corrigé |
| L1 | 🟡 Low | `stderr` de git ignoré silencieusement sur les endpoints HTTP | ✅ corrigé |
| L2 | 🟡 Low | Le contenu d'une notification n'est pas vérifié, seule sa signature l'est | ⚠️ atténué (texte), limite inhérente au modèle |
---
## 2. Détail
### 🟠 M1 — Aucune limitation de débit sur le clone HTTP anonyme
**Fichier** : `cmd/gitfed-web/handlers_git_http.go`
`/{owner}/{repo}.git/info/refs` et `/git-upload-pack` n'avaient, avant ce
correctif, aucune limite d'appels — contrairement au login (`loginByIP`)
et à la réception de notifications (limiteur dédié). Chaque requête peut
déclencher un sous-processus `git upload-pack`, et la fenêtre d'écriture y
est délibérément étendue à 10 minutes (pour laisser le temps à un clone
volumineux sur une connexion lente) — ce qui aggrave l'impact d'un flot de
requêtes plutôt que de le limiter.
**Correctif** : `ratelimit.go` généralisé (`loginLimiter` → `rateLimiter`,
réutilisable) ; nouveau `gitHTTPByIP` à 60 requêtes/minute par IP, appliqué
avant toute autre logique dans les deux handlers. Vérifié manuellement :
la 61ᵉ requête consécutive depuis la même IP reçoit un `429`, un clone
normal (1-2 requêtes) n'est jamais affecté.
---
### 🟠 M2 — Notifications non plafonnées par destinataire
**Fichier** : `internal/store/notifications.go`
Recevoir une notification ne suppose aucune relation préexistante — c'est
volontaire (voir le commentaire de paquet dans `internal/federation/notify.go`).
Mais sans plafond, n'importe quelle instance peut faire grossir
indéfiniment la part d'un utilisateur réel dans la base en lui envoyant de
nombreuses fausses réclamations distinctes (des `Repo`/`Actor` différents
à chaque fois contournent la déduplication, qui ne joue que sur un
doublon exact encore en attente).
**Correctif** : `MaxNotificationsPerPrincipal = 200`. Au-delà, la plus
ancienne notification est évincée pour faire de la place à la nouvelle
(plutôt que de rejeter la nouvelle, ce qui permettrait à du spam de
masquer une notification légitime derrière lui). Testé dans
`TestNotificationCapEvictsOldest`.
---
### 🟠 M3 — Destinataire de notification jamais vérifié comme utilisateur réel
**Fichier** : `internal/federation/notify.go`
La vérification d'origine ne contrôlait que le **domaine** du principal
ciblé (`bob@ailleurs.net` → le suffixe doit être le domaine local), jamais
que `bob` existe réellement. Une notification pour un nom d'utilisateur
inventé était donc acceptée et stockée sans jamais pouvoir être vue par
personne — un vecteur de gonflement de la base par du spam ciblant des
comptes qui n'existent pas.
**Correctif** : `st.GetUser(username)` vérifié avant tout traitement
supplémentaire (avant même la validation de fraîcheur ou l'appel réseau de
vérification de signature — échoue tôt et sans consommer de ressources
inutiles). Testé dans `TestNotifyHandlerRejectsUnknownRecipient` /
`TestNotifyHandlerAcceptsKnownRecipient`.
---
### 🟡 L1 — `stderr` de git ignoré silencieusement
**Fichier** : `cmd/gitfed-web/handlers_git_http.go`
`exec.Command` sans `Stderr` défini envoie la sortie d'erreur vers
`/dev/null` (comportement par défaut de `os/exec` en Go) — pas une fuite
(rien n'était renvoyé au client), mais un angle mort opérationnel : un
`git upload-pack` qui échoue sur un dépôt corrompu ne laissait aucune
trace exploitable, contrairement au chemin SSH équivalent
(`internal/gitexec.Serve`), qui capture correctement `stderr`.
**Correctif** : `stderr` capturé dans un buffer et inclus dans le
`log.Printf` d'erreur côté serveur, jamais renvoyé au client — cohérent
avec le reste du code base.
---
### 🟡 L2 — Le contenu d'une notification n'est pas vérifié
**Fichier** : `internal/federation/notify.go`
La signature prouve que le message vient bien de l'instance qui prétend
l'envoyer — elle ne prouve rien sur la véracité de son **contenu**
(`Repo`, `Role`, `Actor` sont des champs libres). N'importe quelle
instance peut donc envoyer une notification prétendant "vous avez un accès
admin sur tel dépôt" sans que ce soit vrai, à des fins d'ingénierie
sociale (crédibiliser un message de phishing envoyé par un autre canal).
**Analyse d'impact** : c'est une limite *inhérente* à un système
volontairement non-autoritaire (voir le commentaire de paquet) — accepter
une notification ne fait qu'épingler un lien, jamais échanger le moindre
identifiant ni ouvrir de session. Le pire abus reste donc « recevoir un
message trompeur mais inerte », pas une élévation de privilège.
**Traitement retenu** : renforcement du texte affiché sur `/notifications`
pour rendre cette limite explicite (« son contenu n'est pas vérifié
indépendamment... votre accès réel est celui que le propriétaire de cette
instance a réellement défini, peu importe ce qu'affirme une notification »)
plutôt qu'un changement de mécanisme — ajouter une vérification de
contenu supposerait que l'instance réceptrice puisse interroger l'ACL de
l'instance émettrice, ce qui est exactement le couplage fort que ce
système a été conçu pour éviter (voir `DESIGN.md` §3, non-objectifs).
---
## 3. Contrôles vérifiés et jugés sains
| Surface | Constat |
|---|---|
| `repo.Path` dans le clone HTTP | Toujours résolu via `store.GetRepo`, jamais dérivé d'une entrée attaquant — même absence d'injection d'argument que sur le chemin SSH existant. |
| Redirections HTTP pendant la vérification de signature | `guardedDial` s'applique à **chaque** connexion TCP que le `Transport` établit, y compris après une redirection — impossible de contourner le blocage d'IP privée par ce biais. `Fetch` rejette en plus tout document dont le `domain` déclaré ne correspond pas à celui demandé. |
| Exemption CSRF sur `/git-upload-pack` | Le suffixe de chemin qui déclenche l'exemption n'est atteignable que par le handler `handleGitUploadPack` lui-même (aucune autre route ne se termine ainsi) — impossible de faire passer une requête vers un handler différent sous couvert de cette exemption. |
| Écriture via le clone HTTP | Aucune route `git-receive-pack` n'existe côté HTTP, point de vérification trivial mais confirmé (`grep` sur le fichier de routes). |
| Portée des nouvelles méthodes `admin.Ops` (pins, notifications) | Toujours appelées avec `sess.Principal` dérivé de la session HTTP, jamais d'un paramètre soumis par le client — cohérent avec le modèle de confiance déjà documenté du socket RPC (accès total une fois le socket atteint, la vraie barrière est côté HTTP). |
| XSS via un dépôt épinglé | `html/template` échappe `.Domain`/`.Repo` dans le contexte HTML ; le préfixe `https://` est en dur dans le template, donc une valeur du type `javascript:...` saisie comme "domaine" ne produit jamais une URI `javascript:` exploitable, juste un lien cassé. |
| Historique de commits | Format `git log` fixe (non influencé par l'attaquant), chemin de dépôt toujours résolu via le store, taille bornée (`--max-count=200`). |
| Élargissement de l'Ingress (`/.well-known/gitfed.json` exact → `/.well-known/` préfixe) | Sans risque : la surface réelle reste bornée par les deux seules routes enregistrées côté `gitfed-server` (`gitfed.json`, `gitfed-notify`) ; tout le reste sous ce préfixe reçoit un 404 du mux Go, quelle que soit la largeur de la règle Ingress. |
---
## 4. Note opérationnelle (hors code)
`deploy/update.sh` n'applique que `deployment.yaml` — un changement dans
`ingress.yaml`, `service.yaml`, `networkpolicy.yaml` etc. doit être
appliqué manuellement (`kubectl apply -f deploy/k8s/<fichier>.yaml`) après
le déploiement, sans quoi il reste committé sans effet en production.
C'est exactement ce qui s'est produit pour le changement de préfixe
Ingress ci-dessus, repéré et corrigé pendant cette session.