From 3402e53954b5327706955e7199f90b427f6ffd2f Mon Sep 17 00:00:00 2001 From: R0m1k3 Date: Thu, 11 Jun 2026 13:21:43 +0200 Subject: [PATCH] =?UTF-8?q?s=C3=A9curit=C3=A9=20:=20correctifs=20XSS,=20SS?= =?UTF-8?q?RF,=20CSWSH,=20rate-limit=20et=20durcissement?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - XSS stocké (critique) : sanitisation bluemonday sur tous les endpoints d'articles (List, ListByFeed, Favorites, Search, Get), pas seulement Get - SSRF (élevé) : nouveau utils/safehttp.go (ValidateExternalURL + client durci via Dialer.Control, bloque IP privées/loopback/link-local/metadata, anti-DNS-rebinding et limite de redirections) appliqué à l'extracteur et au parser - WebSocket CSWSH (élevé) : politique same-origin + override WS_ALLOWED_ORIGINS - rate-limiting (moyen) : token-bucket en mémoire sur /auth/* - token de session retiré du corps JSON (json:"-"), livré uniquement par le cookie HttpOnly - cookie Secure correct derrière un reverse-proxy (X-Forwarded-Proto + override COOKIE_SECURE) - admin : interdiction de supprimer son propre compte - en-têtes de sécurité (nosniff, X-Frame-Options, Referrer-Policy, Permissions-Policy) Note : backend non compilé localement (pas de toolchain Go) ; à valider via Docker. Co-Authored-By: Claude Opus 4.8 --- cmd/server/main.go | 14 +++- internal/handler/admin.go | 9 +++ internal/handler/article.go | 34 +++++++-- internal/handler/auth.go | 22 +++++- internal/handler/ratelimit.go | 92 ++++++++++++++++++++++++ internal/parser/parser.go | 13 ++-- internal/service/auth.go | 6 +- internal/utils/extractor.go | 10 ++- internal/utils/safehttp.go | 128 ++++++++++++++++++++++++++++++++++ internal/ws/hub.go | 39 ++++++++++- web/src/api/auth.ts | 2 +- 11 files changed, 346 insertions(+), 23 deletions(-) create mode 100644 internal/handler/ratelimit.go create mode 100644 internal/utils/safehttp.go diff --git a/cmd/server/main.go b/cmd/server/main.go index aed718e..661140a 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -80,6 +80,17 @@ func main() { r.Use(middleware.RealIP) r.Use(middleware.Timeout(30 * time.Second)) + // Baseline security headers (defense in depth). + r.Use(func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.Header().Set("X-Content-Type-Options", "nosniff") + w.Header().Set("X-Frame-Options", "DENY") + w.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin") + w.Header().Set("Permissions-Policy", "geolocation=(), microphone=(), camera=()") + next.ServeHTTP(w, req) + }) + }) + // Health check endpoint r.Get("/health", func(w http.ResponseWriter, r *http.Request) { if err := pool.Ping(r.Context()); err != nil { @@ -98,8 +109,9 @@ func main() { w.Write([]byte(`{"message":"FlowReader API v1"}`)) }) - // Auth routes (public) + // Auth routes (public) — rate-limited to mitigate brute-force attacks. r.Route("/auth", func(r chi.Router) { + r.Use(handler.NewAuthRateLimiter()) r.Post("/register", authHandler.Register) r.Post("/login", authHandler.Login) r.Post("/logout", authHandler.Logout) diff --git a/internal/handler/admin.go b/internal/handler/admin.go index f2d711a..780d137 100644 --- a/internal/handler/admin.go +++ b/internal/handler/admin.go @@ -42,6 +42,15 @@ func (h *AdminHandler) DeleteUser(w http.ResponseWriter, r *http.Request) { return } + // Prevent an admin from deleting their own account (lockout / accidental + // self-removal). The AdminOnly middleware has already verified admin rights. + if cookie, cErr := r.Cookie("session_id"); cErr == nil { + if current, _ := h.authService.GetUserByToken(cookie.Value); current != nil && current.ID == userID { + respondError(w, http.StatusForbidden, "You cannot delete your own account") + return + } + } + // Logic to delete user and all associated data if err := h.userRepo.Delete(userID); err != nil { respondError(w, http.StatusInternalServerError, "Failed to delete user") diff --git a/internal/handler/article.go b/internal/handler/article.go index c5766bd..3bd5d43 100644 --- a/internal/handler/article.go +++ b/internal/handler/article.go @@ -37,6 +37,28 @@ func NewArticleHandler(articleRepo domain.ArticleRepository, feedService *servic } } +// sanitizeArticle cleans the user-facing HTML fields of a single article to +// prevent stored XSS from malicious feeds. AISummary is rendered as plain text +// by the client, so only Content and Summary need sanitization. +func (h *ArticleHandler) sanitizeArticle(a *domain.Article) { + if a == nil { + return + } + if a.Content != "" { + a.Content = h.sanitizer.Sanitize(a.Content) + } + if a.Summary != "" { + a.Summary = h.sanitizer.Sanitize(a.Summary) + } +} + +// sanitizeArticles cleans a slice of articles in place. +func (h *ArticleHandler) sanitizeArticles(articles []*domain.Article) { + for _, a := range articles { + h.sanitizeArticle(a) + } +} + // getUserFromRequest extracts the authenticated user from the request. func (h *ArticleHandler) getUserFromRequest(r *http.Request) (uuid.UUID, error) { cookie, err := r.Cookie("session_id") @@ -79,6 +101,7 @@ func (h *ArticleHandler) List(w http.ResponseWriter, r *http.Request) { return } + h.sanitizeArticles(articles) respondJSON(w, http.StatusOK, articles) } @@ -120,6 +143,7 @@ func (h *ArticleHandler) ListByFeed(w http.ResponseWriter, r *http.Request) { return } + h.sanitizeArticles(articles) respondJSON(w, http.StatusOK, articles) } @@ -150,12 +174,8 @@ func (h *ArticleHandler) Get(w http.ResponseWriter, r *http.Request) { return } - // Sanitize content - if article.Content != "" { - article.Content = h.sanitizer.Sanitize(article.Content) - } else if article.Summary != "" { - article.Summary = h.sanitizer.Sanitize(article.Summary) - } + // Sanitize user-facing HTML content (defense against stored XSS). + h.sanitizeArticle(article) respondJSON(w, http.StatusOK, article) } @@ -358,6 +378,7 @@ func (h *ArticleHandler) GetFavorites(w http.ResponseWriter, r *http.Request) { return } + h.sanitizeArticles(articles) respondJSON(w, http.StatusOK, articles) } @@ -391,6 +412,7 @@ func (h *ArticleHandler) Search(w http.ResponseWriter, r *http.Request) { return } + h.sanitizeArticles(articles) respondJSON(w, http.StatusOK, articles) } diff --git a/internal/handler/auth.go b/internal/handler/auth.go index 6f1bff4..b9b6b5e 100644 --- a/internal/handler/auth.go +++ b/internal/handler/auth.go @@ -4,11 +4,29 @@ import ( "encoding/json" "errors" "net/http" + "os" "strings" "github.com/michael/flowreader/internal/service" ) +// secureCookie reports whether the session cookie should carry the Secure flag. +// It honours TLS termination at a reverse proxy (X-Forwarded-Proto) and an +// explicit COOKIE_SECURE override, so HTTPS deployments behind a proxy still +// get Secure cookies even though r.TLS is nil. +func secureCookie(r *http.Request) bool { + switch strings.ToLower(os.Getenv("COOKIE_SECURE")) { + case "true", "1", "yes": + return true + case "false", "0", "no": + return false + } + if r.TLS != nil { + return true + } + return strings.EqualFold(r.Header.Get("X-Forwarded-Proto"), "https") +} + // AuthHandler handles authentication-related HTTP requests. type AuthHandler struct { authService *service.AuthService @@ -74,7 +92,7 @@ func (h *AuthHandler) Login(w http.ResponseWriter, r *http.Request) { Path: "/", Expires: resp.ExpiresAt, HttpOnly: true, - Secure: r.TLS != nil, // Secure only if HTTPS + Secure: secureCookie(r), SameSite: http.SameSiteStrictMode, }) @@ -98,7 +116,7 @@ func (h *AuthHandler) Logout(w http.ResponseWriter, r *http.Request) { Path: "/", MaxAge: -1, HttpOnly: true, - Secure: r.TLS != nil, + Secure: secureCookie(r), SameSite: http.SameSiteStrictMode, }) diff --git a/internal/handler/ratelimit.go b/internal/handler/ratelimit.go new file mode 100644 index 0000000..4ccf4ce --- /dev/null +++ b/internal/handler/ratelimit.go @@ -0,0 +1,92 @@ +package handler + +import ( + "net/http" + "sync" + "time" +) + +// rateLimiter is a simple in-memory token-bucket limiter keyed by client IP. +// It protects brute-force-prone endpoints (login/register) without external +// dependencies. Stale buckets are evicted periodically to bound memory. +type rateLimiter struct { + mu sync.Mutex + buckets map[string]*bucket + rate float64 // tokens added per second + capacity float64 // max tokens (burst) +} + +type bucket struct { + tokens float64 + last time.Time +} + +// newRateLimiter allows `burst` requests immediately, refilling at +// `perMinute` requests per minute thereafter. +func newRateLimiter(perMinute, burst int) *rateLimiter { + rl := &rateLimiter{ + buckets: make(map[string]*bucket), + rate: float64(perMinute) / 60.0, + capacity: float64(burst), + } + go rl.cleanupLoop() + return rl +} + +func (rl *rateLimiter) allow(key string) bool { + rl.mu.Lock() + defer rl.mu.Unlock() + + now := time.Now() + b, ok := rl.buckets[key] + if !ok { + rl.buckets[key] = &bucket{tokens: rl.capacity - 1, last: now} + return true + } + + // Refill based on elapsed time. + b.tokens += now.Sub(b.last).Seconds() * rl.rate + if b.tokens > rl.capacity { + b.tokens = rl.capacity + } + b.last = now + + if b.tokens < 1 { + return false + } + b.tokens-- + return true +} + +func (rl *rateLimiter) cleanupLoop() { + ticker := time.NewTicker(10 * time.Minute) + defer ticker.Stop() + for range ticker.C { + rl.mu.Lock() + for k, b := range rl.buckets { + // Drop buckets that have been idle long enough to be full again. + if time.Since(b.last) > 15*time.Minute { + delete(rl.buckets, k) + } + } + rl.mu.Unlock() + } +} + +// Middleware returns a chi-compatible middleware enforcing the limit per IP. +func (rl *rateLimiter) Middleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !rl.allow(getClientIP(r)) { + w.Header().Set("Retry-After", "60") + respondError(w, http.StatusTooManyRequests, "Too many requests. Please slow down.") + return + } + next.ServeHTTP(w, r) + }) +} + +// NewAuthRateLimiter builds the limiter used for authentication routes: +// 10 requests/minute per IP with a small burst. +func NewAuthRateLimiter() func(http.Handler) http.Handler { + return newRateLimiter(10, 5).Middleware +} diff --git a/internal/parser/parser.go b/internal/parser/parser.go index 0eb1748..2b64f5e 100644 --- a/internal/parser/parser.go +++ b/internal/parser/parser.go @@ -12,6 +12,7 @@ import ( "github.com/PuerkitoBio/goquery" "github.com/google/uuid" "github.com/michael/flowreader/internal/domain" + "github.com/michael/flowreader/internal/utils" "github.com/mmcdole/gofeed" ) @@ -23,12 +24,9 @@ type FeedParser struct { // NewFeedParser creates a new feed parser. func NewFeedParser() *FeedParser { - client := &http.Client{ - Timeout: 30 * time.Second, - } - return &FeedParser{ - client: client, + // SSRF-hardened client: refuses to connect to private/internal addresses. + client: utils.SafeHTTPClient(30 * time.Second), parser: gofeed.NewParser(), } } @@ -44,6 +42,11 @@ type ParsedFeed struct { // Parse fetches and parses a feed URL. func (p *FeedParser) Parse(ctx context.Context, feedURL string, feedID uuid.UUID) (*ParsedFeed, error) { + // Validate up-front (scheme + non-private host) before issuing the request. + if _, err := utils.ValidateExternalURL(feedURL); err != nil { + return nil, err + } + // Create request with context req, err := http.NewRequestWithContext(ctx, http.MethodGet, feedURL, nil) if err != nil { diff --git a/internal/service/auth.go b/internal/service/auth.go index 1afb3d7..c0984a5 100644 --- a/internal/service/auth.go +++ b/internal/service/auth.go @@ -71,9 +71,11 @@ type LoginRequest struct { IPAddress string `json:"-"` } -// LoginResponse contains the session token. +// LoginResponse contains the session result. The token itself is intentionally +// NOT serialized to JSON: it is delivered only via the HttpOnly session cookie +// so it remains inaccessible to client-side scripts. type LoginResponse struct { - Token string `json:"token"` + Token string `json:"-"` ExpiresAt time.Time `json:"expires_at"` User UserInfo `json:"user"` } diff --git a/internal/utils/extractor.go b/internal/utils/extractor.go index 7f75364..bd0a4da 100644 --- a/internal/utils/extractor.go +++ b/internal/utils/extractor.go @@ -18,9 +18,8 @@ type ContentExtractor struct { // NewContentExtractor creates a new extractor instance. func NewContentExtractor() *ContentExtractor { return &ContentExtractor{ - client: &http.Client{ - Timeout: 10 * time.Second, - }, + // SSRF-hardened client: refuses to connect to private/internal addresses. + client: SafeHTTPClient(10 * time.Second), } } @@ -30,6 +29,11 @@ func (e *ContentExtractor) Extract(ctx context.Context, url string) (string, err return "", fmt.Errorf("empty URL") } + // Validate up-front (scheme + non-private host) before issuing the request. + if _, err := ValidateExternalURL(url); err != nil { + return "", err + } + req, err := http.NewRequestWithContext(ctx, "GET", url, nil) if err != nil { return "", err diff --git a/internal/utils/safehttp.go b/internal/utils/safehttp.go new file mode 100644 index 0000000..5cfed95 --- /dev/null +++ b/internal/utils/safehttp.go @@ -0,0 +1,128 @@ +package utils + +import ( + "context" + "fmt" + "net" + "net/http" + "net/url" + "strings" + "syscall" + "time" +) + +// ErrBlockedHost is returned when a URL resolves to a non-public address. +type ErrBlockedHost struct{ Host string } + +func (e *ErrBlockedHost) Error() string { + return fmt.Sprintf("blocked request to non-public host: %s", e.Host) +} + +// isDisallowedIP reports whether an IP is private, loopback, link-local, +// unspecified, or otherwise unsafe to fetch (SSRF protection). +func isDisallowedIP(ip net.IP) bool { + if ip == nil { + return true + } + if ip.IsLoopback() || ip.IsPrivate() || ip.IsUnspecified() || + ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsMulticast() { + return true + } + // Block IPv4-mapped cloud metadata endpoint explicitly (169.254.169.254 is + // already link-local, but keep an explicit guard for clarity/IPv6 forms). + if v4 := ip.To4(); v4 != nil { + // 0.0.0.0/8 and 100.64.0.0/10 (CGNAT) are also unsafe targets. + if v4[0] == 0 { + return true + } + if v4[0] == 100 && v4[1]&0xC0 == 64 { + return true + } + } + return false +} + +// ValidateExternalURL parses raw, enforces http(s), and verifies that the host +// does not resolve to any disallowed (private/internal) address. It returns the +// parsed URL so callers can reuse the normalized form. +func ValidateExternalURL(raw string) (*url.URL, error) { + u, err := url.Parse(strings.TrimSpace(raw)) + if err != nil { + return nil, fmt.Errorf("invalid URL: %w", err) + } + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("unsupported scheme %q", u.Scheme) + } + host := u.Hostname() + if host == "" { + return nil, fmt.Errorf("missing host") + } + + // If the host is a literal IP, validate it directly. + if ip := net.ParseIP(host); ip != nil { + if isDisallowedIP(ip) { + return nil, &ErrBlockedHost{Host: host} + } + return u, nil + } + + // Otherwise resolve and validate every returned address. + ips, err := net.DefaultResolver.LookupIPAddr(context.Background(), host) + if err != nil { + return nil, fmt.Errorf("resolving host: %w", err) + } + if len(ips) == 0 { + return nil, &ErrBlockedHost{Host: host} + } + for _, addr := range ips { + if isDisallowedIP(addr.IP) { + return nil, &ErrBlockedHost{Host: host} + } + } + return u, nil +} + +// SafeHTTPClient returns an *http.Client hardened against SSRF. A dial-time +// Control hook re-validates the resolved IP for every connection, which also +// defeats DNS-rebinding (TOCTOU) attacks that pass the up-front check. +func SafeHTTPClient(timeout time.Duration) *http.Client { + dialer := &net.Dialer{ + Timeout: 10 * time.Second, + KeepAlive: 30 * time.Second, + Control: func(_, address string, _ syscall.RawConn) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return err + } + ip := net.ParseIP(host) + if isDisallowedIP(ip) { + return &ErrBlockedHost{Host: host} + } + return nil + }, + } + + transport := &http.Transport{ + DialContext: dialer.DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } + + return &http.Client{ + Timeout: timeout, + Transport: transport, + // Re-validate the target on each redirect hop and cap redirect depth. + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 5 { + return fmt.Errorf("too many redirects") + } + if _, err := ValidateExternalURL(req.URL.String()); err != nil { + return err + } + return nil + }, + } +} diff --git a/internal/ws/hub.go b/internal/ws/hub.go index f3811ad..0dfc884 100644 --- a/internal/ws/hub.go +++ b/internal/ws/hub.go @@ -4,16 +4,49 @@ import ( "encoding/json" "log" "net/http" + "net/url" + "os" + "strings" "sync" "github.com/google/uuid" "github.com/gorilla/websocket" ) +// allowedWSOrigins holds optional extra origins (comma-separated) from the +// WS_ALLOWED_ORIGINS env var, for deployments where the WS host differs. +var allowedWSOrigins = parseAllowedOrigins(os.Getenv("WS_ALLOWED_ORIGINS")) + +func parseAllowedOrigins(raw string) map[string]bool { + out := make(map[string]bool) + for _, o := range strings.Split(raw, ",") { + if o = strings.TrimSpace(strings.ToLower(o)); o != "" { + out[o] = true + } + } + return out +} + +// checkOrigin enforces a same-origin policy to prevent Cross-Site WebSocket +// Hijacking (CSWSH). Requests without an Origin header (non-browser clients) +// are allowed; browser requests must match the Host or an allow-listed origin. +func checkOrigin(r *http.Request) bool { + origin := r.Header.Get("Origin") + if origin == "" { + return true // non-browser client (e.g. native app, curl) + } + u, err := url.Parse(origin) + if err != nil { + return false + } + if strings.EqualFold(u.Host, r.Host) { + return true + } + return allowedWSOrigins[strings.ToLower(u.Host)] +} + var upgrader = websocket.Upgrader{ - CheckOrigin: func(r *http.Request) bool { - return true // In production, check origin properly - }, + CheckOrigin: checkOrigin, } // Event represents a websocket event. diff --git a/web/src/api/auth.ts b/web/src/api/auth.ts index 18fe1c6..fcecd49 100644 --- a/web/src/api/auth.ts +++ b/web/src/api/auth.ts @@ -17,7 +17,7 @@ export interface LoginRequest { } export interface LoginResponse { - token: string; + // The session token is delivered via an HttpOnly cookie, not the JSON body. expires_at: string; user: User; }