From 841571ea083c465efa4038a40d007b9d6c3bb9e8 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 28 Sep 2026 20:03:35 +0000 Subject: [PATCH] Check the origin's host and port; cap the size at 1 GiB (closes #61) access_control_allow_origin now needs a host name (ASCII letters, digits, hyphens, dots) or an IP address, and a port, when given, from 1 to 65535, read with net/url, net/netip and strconv. Two hosts, an empty port, no host, a bad port or a non-ASCII host name abort startup, as a * inside the value already did. upstream_max_response_size above 1 GiB aborts startup: the image processor reads one byte past the limit, which wrapped negative at the largest 64-bit value, and a response is held whole in memory. config.example.yml states the maximum. Model: opus-5-5 --- TODO.md | 8 ++-- config.example.yml | 3 +- internal/config/config.go | 81 ++++++++++++++++++++++++++++++++------- 3 files changed, 74 insertions(+), 18 deletions(-) diff --git a/TODO.md b/TODO.md index 6234c46..848a58e 100644 --- a/TODO.md +++ b/TODO.md @@ -63,9 +63,11 @@ exhaustion `upstream_fetch_timeout` (default `30s`), `upstream_max_response_size` (default 50 MiB) and `downstream_timeout` (default `60s`, both the server's write timeout and the per-request timeout); each has a - `PIXA_` variable; durations are positive Go duration strings, sizes a - whole number of bytes; an invalid value aborts startup naming the key - and the value; documented in `config.example.yml` and `README.md`. + `PIXA_` variable; durations are positive Go duration strings, the size a + whole number of bytes up to 1 GiB, the origin `*` or one scheme, host + name or IP address and optional port; an invalid value aborts startup + naming the key and the value; documented in `config.example.yml` and + `README.md`. - 2026-09-28 cache stats report real numbers (closes #56): `Cache.Stats` counts the cached source images and processed variants (`source_content` plus `variant_content`) and takes their size from `Cache.UsageBytes`, diff --git a/config.example.yml b/config.example.yml index 686b45d..e08a8d1 100644 --- a/config.example.yml +++ b/config.example.yml @@ -74,7 +74,8 @@ upstream_connections_per_host: 20 # Time allowed for one fetch from an upstream host (default: 30s) upstream_fetch_timeout: 30s -# Largest upstream response accepted, in bytes (default: 52428800, 50 MiB) +# Largest upstream response accepted, in bytes, at most 1073741824 +# (1 GiB) (default: 52428800, 50 MiB) upstream_max_response_size: 52428800 # Time allowed for answering one client request, the upstream fetch diff --git a/internal/config/config.go b/internal/config/config.go index 4f63900..1c17369 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -78,6 +78,7 @@ var ( errEmptyEntry = errors.New("contains an empty entry") errNotAValidURL = errors.New("not a valid URL") errPortOutOfRange = errors.New("outside the valid port range") + errSizeOutOfRange = errors.New("outside the accepted range") errTooFewConnections = errors.New("must be at least 1") errValueTooShort = errors.New("value too short") errPlaceholderKey = errors.New( @@ -578,10 +579,9 @@ func (c *Config) validate() error { settingName(keyCacheMaxBytes), c.CacheMaxBytes, errMustNotBeNegative) } - if c.UpstreamMaxResponseSize <= 0 { - return fmt.Errorf("%s: value %d %w", - settingName(keyUpstreamMaxResponseSize), c.UpstreamMaxResponseSize, - errMustBePositive) + err = c.validateUpstreamMaxResponseSize() + if err != nil { + return err } for _, host := range c.AllowlistHosts { @@ -608,29 +608,82 @@ func (c *Config) validate() error { return c.validateAccessControlAllowOrigin() } +// validateUpstreamMaxResponseSize checks that upstream_max_response_size +// is from 1 byte to 1 GiB. An upstream response is read whole into +// memory, and the image processor reads one byte past this limit, which +// must not overflow. +func (c *Config) validateUpstreamMaxResponseSize() error { + const maxUpstreamMaxResponseSize = 1 << 30 // 1 GiB + if c.UpstreamMaxResponseSize < 1 || + c.UpstreamMaxResponseSize > maxUpstreamMaxResponseSize { + return fmt.Errorf("%s: value %d is %w 1-%d", + settingName(keyUpstreamMaxResponseSize), c.UpstreamMaxResponseSize, + errSizeOutOfRange, maxUpstreamMaxResponseSize) + } + + return nil +} + // validateAccessControlAllowOrigin checks that access_control_allow_origin -// is "*" or one origin, a scheme and host with nothing after them, as -// browsers send it in the Origin header. Anything else, such as a bare -// hostname or a trailing slash, would match no request. An origin with a -// "*" inside, such as https://*example.com, is refused because the CORS -// middleware reads that "*" as a pattern and would let other sites read -// responses. +// is "*" or one origin as browsers send it in the Origin header: a scheme, +// a host name or IP address, and optionally a port from 1 to 65535, with +// nothing after them. Anything else, such as a bare hostname or a trailing +// slash, would match no request. A "*" is not allowed in a host name, so +// https://*example.com is refused; the CORS middleware would read that +// "*" as a pattern and let other sites read responses. func (c *Config) validateAccessControlAllowOrigin() error { origin := c.AccessControlAllowOrigin if origin == "*" { return nil } + errOrigin := fmt.Errorf("%s: value %q is %w", + settingName(keyAccessControlAllowOrigin), origin, errNotAnOrigin) + + // url.Parse accepts an empty port, as in https://example.com:, and + // then reports no port, so a trailing colon is refused here. parsed, err := url.Parse(origin) - if err != nil || parsed.Host == "" || parsed.Scheme+"://"+parsed.Host != origin || - strings.Contains(origin, "*") { - return fmt.Errorf("%s: value %q is %w", - settingName(keyAccessControlAllowOrigin), origin, errNotAnOrigin) + if err != nil || parsed.Scheme+"://"+parsed.Host != origin || + strings.HasSuffix(origin, ":") { + return errOrigin + } + + host := parsed.Hostname() + + _, err = netip.ParseAddr(host) + if err != nil && !isHostName(host) { + return errOrigin + } + + // A port is a 16-bit number, and 0 is not a port a browser sends. + if parsed.Port() != "" { + port, err := strconv.ParseUint(parsed.Port(), 10, 16) + if err != nil || port == 0 { + return errOrigin + } } return nil } +// isHostName reports whether host is not empty and has only ASCII +// letters, digits, hyphens and dots. +func isHostName(host string) bool { + if host == "" { + return false + } + + for _, r := range host { + isLetterOrDigit := 'a' <= r && r <= 'z' || 'A' <= r && r <= 'Z' || + '0' <= r && r <= '9' + if !isLetterOrDigit && r != '-' && r != '.' { + return false + } + } + + return true +} + // validateAllowlistHost checks that an allowlist_hosts entry is a bare // hostname, optionally with a leading dot for suffix matching. URLs, // paths, and whitespace indicate a misconfigured entry. An entry with