Bild-Upload: animierte GIFs erhalten statt aufs erste Frame reduzieren

Animierte GIFs (mehr als 1 Frame) werden jetzt unverändert durchgereicht statt via Decode+Re-Encode auf das erste Frame reduziert. Da die Bytes dabei nicht mehr sanitisiert werden, zwei Guardrails: - gif.DecodeAll läuft über einen io.LimitReader (16 MB) und prüft danach ein Frame-Budget (1000) sowie ein Gesamtpixel-Budget (100 MP) -> bremst den DoS-Vektor (GIF-Bombe mit vielen Frames). copyUpload schreibt die Bytes unter serverseitig erzeugtem Namen und deckelt hart bei 16 MB. - noSniff-Middleware setzt X-Content-Type-Options: nosniff auf /static/*, damit der Browser eine durchgereichte Datei nicht als HTML/JS deutet (Polyglot/Stored-XSS). Standbilder (JPEG/PNG/Einzelbild-GIF) behalten den skalierenden Re-Encode-Pfad und damit die Sanitisierung. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-10 07:06:54 +02:00
parent ad2c08d93a
commit 925b00edf5
2 changed files with 93 additions and 18 deletions
@@ -425,11 +425,22 @@ func handleVote(w http.ResponseWriter, r *http.Request) {
 	writeTally(w, pid, uid)
 }

-// storeImage skaliert ein hochgeladenes Bild auf max. 1024px und speichert es unter static/media.
-// Hinweis: animierte GIFs werden derzeit auf das erste Frame reduziert.
-func storeImage(fh *multipart.FileHeader) (string, error) {
-	const maxSize = 1024
+const (
+	maxImageSize   = 1024         // Zielkante beim Skalieren
+	maxImagePixels = 50_000_000   // pro Frame: Schutz vor Decompression-Bomb
+	maxGIFBytes    = 16 << 20     // Obergrenze für durchgereichte GIFs
+	maxGIFFrames   = 1000         // animiertes GIF: Frame-Budget
+	maxGIFPixels   = 100_000_000  // animiertes GIF: Summe aller Frame-Pixel
+)

+// storeImage speichert ein hochgeladenes Bild unter static/media.
+// Standbilder (JPEG/PNG/Einzelbild-GIF) werden auf max. 1024px skaliert und
+// dabei durch Decode+Re-Encode sanitisiert. Animierte GIFs würden durch das
+// Re-Encode auf das erste Frame reduziert -> sie werden stattdessen unverändert
+// durchgereicht. Die Bytes sind dann NICHT sanitisiert; deshalb werden Medien
+// mit X-Content-Type-Options: nosniff ausgeliefert (siehe routes()), und Frame-
+// sowie Gesamtpixel-Budget begrenzen den DoS-Vektor von gif.DecodeAll.
+func storeImage(fh *multipart.FileHeader) (string, error) {
 	src, err := fh.Open()
 	if err != nil {
 		return "", err
@@ -439,19 +450,52 @@ func storeImage(fh *multipart.FileHeader) (string, error) {
 	// Dimensionen aus dem Header prüfen, bevor wir die vollen Pixel allokieren:
 	// eine kleine Datei kann riesige Maße deklarieren (Decompression-Bomb) und
 	// sonst beim Decode den Speicher sprengen.
-	const maxPixels = 50_000_000 // ~50 MP
-	cfg, _, err := image.DecodeConfig(src)
+	cfg, format, err := image.DecodeConfig(src)
 	if err != nil {
 		return "", err
 	}
-	if cfg.Width <= 0 || cfg.Height <= 0 || cfg.Width*cfg.Height > maxPixels {
+	if cfg.Width <= 0 || cfg.Height <= 0 || cfg.Width*cfg.Height > maxImagePixels {
 		return "", fmt.Errorf("bild zu groß: %dx%d", cfg.Width, cfg.Height)
 	}
-	if _, err := src.Seek(0, io.SeekStart); err != nil {
+
+	if err := os.MkdirAll("static/media", 0o755); err != nil {
 		return "", err
 	}

-	img, format, err := image.Decode(src)
+	if format == "gif" {
+		// LimitReader deckelt den von DecodeAll gelesenen (und damit allokierten)
+		// Speicher unabhängig von einer evtl. gelogenen Header-Größe.
+		if _, err := src.Seek(0, io.SeekStart); err != nil {
+			return "", err
+		}
+		g, err := gif.DecodeAll(io.LimitReader(src, maxGIFBytes))
+		if err != nil {
+			return "", err
+		}
+		if len(g.Image) > 1 {
+			if len(g.Image) > maxGIFFrames {
+				return "", fmt.Errorf("animiertes GIF: zu viele Frames (%d)", len(g.Image))
+			}
+			total := 0
+			for _, fr := range g.Image {
+				b := fr.Bounds()
+				total += b.Dx() * b.Dy()
+				if total > maxGIFPixels {
+					return "", fmt.Errorf("animiertes GIF zu groß")
+				}
+			}
+			if _, err := src.Seek(0, io.SeekStart); err != nil {
+				return "", err
+			}
+			return copyUpload(src, "gif")
+		}
+		// Einzelbild-GIF: weiter in den Skalierungspfad unten.
+	}
+
+	if _, err := src.Seek(0, io.SeekStart); err != nil {
+		return "", err
+	}
+	img, _, err := image.Decode(src)
 	if err != nil {
 		return "", err
 	}
@@ -460,11 +504,11 @@ func storeImage(fh *multipart.FileHeader) (string, error) {
 	width, height := b.Dx(), b.Dy()

 	nw, nh := width, height
-	if width > maxSize || height > maxSize {
+	if width > maxImageSize || height > maxImageSize {
 		if width > height {
-			nw, nh = maxSize, height*maxSize/width
+			nw, nh = maxImageSize, height*maxImageSize/width
 		} else {
-			nw, nh = width*maxSize/height, maxSize
+			nw, nh = width*maxImageSize/height, maxImageSize
 		}
 	}

@@ -472,10 +516,6 @@ func storeImage(fh *multipart.FileHeader) (string, error) {
 	if format == "gif" {
 		ext = "gif"
 	}
-
-	if err := os.MkdirAll("static/media", 0o755); err != nil {
-		return "", err
-	}
 	name := fmt.Sprintf("static/media/%d-%d.%s", time.Now().Unix(), rand.IntN(999999), ext)

 	dst := image.NewRGBA(image.Rect(0, 0, nw, nh))
@@ -498,3 +538,27 @@ func storeImage(fh *multipart.FileHeader) (string, error) {
 	}
 	return name, nil
 }
+
+// copyUpload schreibt die Originalbytes (max. maxGIFBytes) unter einem
+// serverseitig erzeugten Namen nach static/media. Der hochgeladene Dateiname
+// fließt nie ein (kein Path-Traversal).
+func copyUpload(src io.Reader, ext string) (string, error) {
+	name := fmt.Sprintf("static/media/%d-%d.%s", time.Now().Unix(), rand.IntN(999999), ext)
+	out, err := os.Create(name)
+	if err != nil {
+		return "", err
+	}
+	defer out.Close()
+
+	// maxGIFBytes+1 lesen, um ein Überschreiten sicher zu erkennen.
+	n, err := io.Copy(out, io.LimitReader(src, maxGIFBytes+1))
+	if err != nil {
+		os.Remove(name)
+		return "", err
+	}
+	if n > maxGIFBytes {
+		os.Remove(name)
+		return "", fmt.Errorf("datei zu groß")
+	}
+	return name, nil
+}
@@ -65,8 +65,10 @@ func routes() http.Handler {
 		r.Post("/user/delete", handleUserDelete)
 	})

-	// Hochgeladene Medien + alte statische Assets
-	r.Handle("/static/*", http.StripPrefix("/static/", http.FileServer(http.Dir("static"))))
+	// Hochgeladene Medien + alte statische Assets. nosniff verhindert, dass der
+	// Browser eine durchgereichte (nicht re-kodierte) Datei als HTML/JS
+	// interpretiert -> schließt Polyglot/Stored-XSS über Bild-Uploads.
+	r.Handle("/static/*", http.StripPrefix("/static/", noSniff(http.FileServer(http.Dir("static")))))

 	// Neues JS-Frontend
 	r.Handle("/*", http.FileServer(http.Dir("web")))
@@ -74,6 +76,15 @@ func routes() http.Handler {
 	return r
 }

+// noSniff setzt X-Content-Type-Options: nosniff, damit der Browser den
+// deklarierten Content-Type nicht überstimmt (Schutz vor Content-Sniffing).
+func noSniff(h http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("X-Content-Type-Options", "nosniff")
+		h.ServeHTTP(w, r)
+	})
+}
+
 func writeJSON(w http.ResponseWriter, status int, v any) {
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(status)