From e8558c1cdd647c77f6b56051f7d35821f2a206a8 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 28 Sep 2026 12:29:38 +0000 Subject: [PATCH] fix: validate dimensions and fit mode on encrypted URLs (closes #62) The encrypted /v1/e/ route used the decrypted payload unchecked, so a token could request an over-limit size or an unknown fit mode; the generator turned unparseable numbers into 0. imgcache.ValidateDimension alone holds the MaxDimension bound and is used by the path parser, by the new ValidateImageRequest (which adds ValidateFitMode) and by the generator. Both image routes call ValidateImageRequest, so each answers 400. The generator answers 400 naming the field for a width or height that is not a number or fails that check, a quality that is not a number from 1 to 100, a ttl that is not a number from 0 to the largest the expiry calculation can hold, or an unknown fit. Empty quality is 85; empty ttl never expires. The form's size inputs stop at 8192. Model: opus-4-8 (implementation); opus-5-5 (rework) --- TODO.md | 14 +++ internal/handlers/auth.go | 149 ++++++++++++++++++++++++++---- internal/handlers/image.go | 19 ++-- internal/handlers/imageenc.go | 13 +++ internal/imgcache/imgcache.go | 18 ++++ internal/imgcache/urlparser.go | 29 +++++- internal/templates/generator.html | 4 +- 7 files changed, 216 insertions(+), 30 deletions(-) diff --git a/TODO.md b/TODO.md index e859d2e..d4989f0 100644 --- a/TODO.md +++ b/TODO.md @@ -89,6 +89,20 @@ exhaustion (IPv4-mapped forms covered); enforcement stays in the dial-time re-resolution so the DNS-rebinding window remains closed; documented in `README.md` and `config.example.yml`. +- 2026-09-21 validate dimensions and fit mode on the encrypted-URL + route and the token generator (closes #62): `imgcache.ValidateDimension` + alone holds the `MaxDimension` bound and is used by the path parser, by + the new `ValidateImageRequest` (which also applies `ValidateFitMode`) + and by the generator; both the `/v1/image/` and `/v1/e/` routes call + `ValidateImageRequest`, so an over-limit size or an unknown fit mode is a + 400 rather than an out-of-memory or a 500 from the processor; the URL + generator answers 400 naming the field for a `width` or `height` that is + not a number or fails the shared check, a `quality` that is not a number + from 1 to 100, a `ttl` that is not a number from 0 to the largest number + of seconds the expiry calculation can hold, or an unknown `fit`; an empty + `quality` is 85 and + an empty `ttl` never expires; the form's width and height inputs stop at + 8192 - 2026-09-21 http.Server hardening (closes #92): added `HTTPReadHeaderTimeout` (10s, bounds the slowloris header dribble) and `HTTPIdleTimeout` (120s, bounds keep-alive reuse) alongside the diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 5ea2a32..4edf22f 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -1,8 +1,12 @@ package handlers import ( + "bytes" "crypto/subtle" + "errors" + "fmt" "html/template" + "math" "net/http" "net/url" "strconv" @@ -14,6 +18,19 @@ import ( "sneak.berlin/go/pixa/internal/templates" ) +// errInvalidFormField reports a generator form field whose value is +// non-numeric or out of range. The offending field name is wrapped in so the +// response can name it. +var errInvalidFormField = errors.New("invalid") + +// Bounds for the generator's quality and ttl fields. maxTTL is in seconds: +// the expiry calculation time.Duration(ttl) * time.Second overflows above it. +const ( + minQuality = 1 + maxQuality = 100 + maxTTL = int(math.MaxInt64 / time.Second) +) + // HandleRoot serves the login page or generator page based on authentication state. func (s *Handlers) HandleRoot() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { @@ -101,18 +118,26 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { // Validate source URL parsed, err := url.Parse(sourceURL) if err != nil || parsed.Host == "" { - s.renderGeneratorWithForm(w, r, "Invalid source URL", r.Form) + s.renderGeneratorWithForm(w, r, "Invalid source URL", r.Form, + http.StatusBadRequest) return } - payload, expiresAt, ttl := buildGeneratePayload(parsed, r.Form) + payload, expiresAt, ttl, err := buildGeneratePayload(parsed, r.Form) + if err != nil { + s.renderGeneratorWithForm(w, r, err.Error(), r.Form, + http.StatusBadRequest) + + return + } // Generate encrypted token token, err := s.encGen.Generate(payload) if err != nil { s.log.Error("failed to generate encrypted URL", "error", err) - s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form) + s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form, + http.StatusInternalServerError) return } @@ -140,17 +165,40 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { } // buildGeneratePayload parses the numeric form fields and assembles the -// encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0). +// encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0). A +// non-numeric or out-of-range width, height, quality or ttl, or an +// unrecognized fit mode, is a client error naming the offending field. The +// format field is passed through unchecked. func buildGeneratePayload( parsed *url.URL, form url.Values, -) (*encurl.Payload, time.Time, int) { - width, _ := strconv.Atoi(form.Get("width")) - height, _ := strconv.Atoi(form.Get("height")) - quality, _ := strconv.Atoi(form.Get("quality")) - ttl, _ := strconv.Atoi(form.Get("ttl")) +) (*encurl.Payload, time.Time, int, error) { + width, err := parseFormDimension(form, "width") + if err != nil { + return nil, time.Time{}, 0, err + } - if quality <= 0 { - quality = 85 + height, err := parseFormDimension(form, "height") + if err != nil { + return nil, time.Time{}, 0, err + } + + quality, err := parseFormInt(form, "quality", + encurl.DefaultQuality, minQuality, maxQuality) + if err != nil { + return nil, time.Time{}, 0, err + } + + ttl, err := parseFormInt(form, "ttl", 0, 0, maxTTL) + if err != nil { + return nil, time.Time{}, 0, err + } + + fitMode := imgcache.FitMode(form.Get("fit")) + + err = imgcache.ValidateFitMode(fitMode) + if err != nil { + return nil, time.Time{}, 0, + fmt.Errorf("%w: %s", imgcache.ErrInvalidFitMode, form.Get("fit")) } var ( @@ -171,11 +219,57 @@ func buildGeneratePayload( Height: height, Format: imgcache.ImageFormat(form.Get("format")), Quality: quality, - FitMode: imgcache.FitMode(form.Get("fit")), + FitMode: fitMode, ExpiresAt: expiresAtUnix, } - return payload, expiresAt, ttl + return payload, expiresAt, ttl, nil +} + +// parseFormDimension reads an optional width or height form field. An empty +// value means "original size" (0). A non-numeric value, or one +// imgcache.ValidateDimension rejects, is an error naming the field. +func parseFormDimension(form url.Values, field string) (int, error) { + raw := form.Get(field) + if raw == "" { + return 0, nil + } + + value, err := strconv.Atoi(raw) + if err != nil { + return 0, fmt.Errorf("%w %s: not a number", errInvalidFormField, field) + } + + err = imgcache.ValidateDimension(field, value) + if err != nil { + return 0, err + } + + return value, nil +} + +// parseFormInt reads an optional integer form field, returning def when the +// field is empty and an error naming the field when the value is non-numeric +// or outside minValue to maxValue. +func parseFormInt( + form url.Values, field string, def, minValue, maxValue int, +) (int, error) { + raw := form.Get(field) + if raw == "" { + return def, nil + } + + value, err := strconv.Atoi(raw) + if err != nil { + return 0, fmt.Errorf("%w %s: not a number", errInvalidFormField, field) + } + + if value < minValue || value > maxValue { + return 0, fmt.Errorf("%w %s: must be from %d to %d", + errInvalidFormField, field, minValue, maxValue) + } + + return value, nil } // generatorData holds template data for the generator page. @@ -215,6 +309,15 @@ func (s *Handlers) renderLogin( func (s *Handlers) renderGenerator( w http.ResponseWriter, r *http.Request, data *generatorData, +) { + s.renderGeneratorStatus(w, r, data, http.StatusOK) +} + +// renderGeneratorStatus renders the generator page with an explicit HTTP +// status; a rejected form uses 400. The page is rendered into a buffer before +// the status is written, so a template failure can still answer 500. +func (s *Handlers) renderGeneratorStatus( + w http.ResponseWriter, r *http.Request, data *generatorData, status int, ) { w.Header().Set("Content-Type", "text/html; charset=utf-8") @@ -224,17 +327,29 @@ func (s *Handlers) renderGenerator( data.CSRFField = csrfField(r) - err := templates.Render(w, "generator.html", data) + var page bytes.Buffer + + err := templates.Render(&page, "generator.html", data) if err != nil { s.log.Error("failed to render generator template", "error", err) http.Error(w, "Internal server error", http.StatusInternalServerError) + + return + } + + w.WriteHeader(status) + + _, err = page.WriteTo(w) + if err != nil { + s.log.Error("failed to write generator page", "error", err) } } func (s *Handlers) renderGeneratorWithForm( - w http.ResponseWriter, r *http.Request, errorMsg string, form url.Values, + w http.ResponseWriter, r *http.Request, errorMsg string, + form url.Values, status int, ) { - s.renderGenerator(w, r, &generatorData{ + s.renderGeneratorStatus(w, r, &generatorData{ Error: errorMsg, FormURL: form.Get("url"), FormWidth: form.Get("width"), @@ -243,7 +358,7 @@ func (s *Handlers) renderGeneratorWithForm( FormQuality: form.Get("quality"), FormFit: form.Get("fit"), FormTTL: form.Get("ttl"), - }) + }, status) } func (s *Handlers) buildGeneratedURL(r *http.Request, token, format string) string { diff --git a/internal/handlers/image.go b/internal/handlers/image.go index c3de6b5..7411e56 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -110,13 +110,6 @@ func (s *Handlers) parseImageRequest( if fit := query.Get("fit"); fit != "" { req.FitMode = imgcache.FitMode(fit) - - fitErr := imgcache.ValidateFitMode(req.FitMode) - if fitErr != nil { - s.respondError(w, "invalid fit mode: "+fit, http.StatusBadRequest) - - return nil, false - } } // Default quality if not set @@ -129,6 +122,18 @@ func (s *Handlers) parseImageRequest( req.FitMode = imgcache.FitCover } + // Enforce dimension and fit-mode bounds, shared with the encrypted-URL + // route. Dimensions are already bounded by the path parser above; this + // also rejects an unrecognized fit mode with 400 instead of letting it + // reach the processor as a 500. + err = imgcache.ValidateImageRequest(req) + if err != nil { + s.respondError(w, "invalid image request: "+err.Error(), + http.StatusBadRequest) + + return nil, false + } + return req, true } diff --git a/internal/handlers/imageenc.go b/internal/handlers/imageenc.go index e01b2cd..8089f64 100644 --- a/internal/handlers/imageenc.go +++ b/internal/handlers/imageenc.go @@ -50,6 +50,19 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { // Convert payload to ImageRequest req := payload.ToImageRequest() + // Apply the same dimension and fit-mode bounds as the plain image + // route: a sealed payload is trusted for its origin, not for staying + // within limits, so an over-limit size or unknown fit mode is a 400 + // here rather than an out-of-memory or a 500 from the processor. + err = imgcache.ValidateImageRequest(req) + if err != nil { + s.log.Debug("encrypted URL failed validation", "error", err) + s.respondError(w, "invalid encrypted URL: "+err.Error(), + http.StatusBadRequest) + + return + } + // Log the request s.log.Debug("encrypted image request", "host", req.SourceHost, diff --git a/internal/imgcache/imgcache.go b/internal/imgcache/imgcache.go index e3dd61e..7fb069b 100644 --- a/internal/imgcache/imgcache.go +++ b/internal/imgcache/imgcache.go @@ -59,6 +59,24 @@ func ValidateFitMode(fit FitMode) error { } } +// ValidateImageRequest checks a request's width and height with +// ValidateDimension and its fit mode with ValidateFitMode. Both the plain +// /v1/image/ route and the encrypted /v1/e/ route validate through this +// function so a request from either source enforces identical bounds. +func ValidateImageRequest(req *ImageRequest) error { + err := ValidateDimension("width", req.Size.Width) + if err != nil { + return err + } + + err = ValidateDimension("height", req.Size.Height) + if err != nil { + return err + } + + return ValidateFitMode(req.FitMode) +} + // ImageRequest represents a request for a processed image type ImageRequest struct { // SourceHost is the origin host (e.g., "cdn.example.com") diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index 6a391ee..a1a424c 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -23,6 +23,21 @@ var ( // MaxDimension is the maximum allowed width or height. const MaxDimension = 8192 +// ValidateDimension checks one requested width or height; name ("width" or +// "height") appears in the error. 0 means "original size" and is valid. +func ValidateDimension(name string, value int) error { + if value < 0 { + return fmt.Errorf("%w: %s is negative", ErrInvalidSize, name) + } + + if value > MaxDimension { + return fmt.Errorf("%w: %s is above %d", + ErrDimensionTooLarge, name, MaxDimension) + } + + return nil +} + // sizeFormatRegex matches patterns like "800x600.webp", "0x0.jpeg", "orig.png" var sizeFormatRegex = regexp.MustCompile(`^(\d+)x(\d+)\.(\w+)$|^(orig)\.(\w+)$`) @@ -225,14 +240,20 @@ func parseSizeFormat(s string) (Size, ImageFormat, error) { return Size{}, "", ErrInvalidSize } - if width > MaxDimension || height > MaxDimension { - return Size{}, "", ErrDimensionTooLarge - } - size = Size{Width: width, Height: height} formatStr = matches[3] } + err := ValidateDimension("width", size.Width) + if err != nil { + return Size{}, "", err + } + + err = ValidateDimension("height", size.Height) + if err != nil { + return Size{}, "", err + } + format, err := parseFormat(formatStr) if err != nil { return Size{}, "", err diff --git a/internal/templates/generator.html b/internal/templates/generator.html index d82424d..e92ced2 100644 --- a/internal/templates/generator.html +++ b/internal/templates/generator.html @@ -73,7 +73,7 @@ id="width" name="width" min="0" - max="10000" + max="8192" value="{{if .FormWidth}}{{.FormWidth}}{{else}}0{{end}}" placeholder="0 = original" class="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500" @@ -88,7 +88,7 @@ id="height" name="height" min="0" - max="10000" + max="8192" value="{{if .FormHeight}}{{.FormHeight}}{{else}}0{{end}}" placeholder="0 = original" class="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500"