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"