From e836e88b8fb343be5444483e6e89c859f0566417 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 28 Sep 2026 20:25:42 +0000 Subject: [PATCH] Accept only an origin written exactly as a browser sends it (closes #61) access_control_allow_origin is now "*", or http or https, a host that is an IP address as net/netip writes it (IPv6 in brackets) or a lowercase host name whose last part contains a letter, and an optional port 1-65535 with no leading zero that is not the scheme's default. The value must equal the origin rebuilt from those parts; anything else aborts startup naming the key, its variable and the value. README.md and config.example.yml say an origin is scheme, host and optional port, exactly as the browser sends it. Model: opus-5-5 --- README.md | 5 +-- TODO.md | 6 ++-- config.example.yml | 3 +- internal/config/config.go | 70 +++++++++++++++++++-------------------- 4 files changed, 43 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index af1b0e1..3c0df38 100644 --- a/README.md +++ b/README.md @@ -249,8 +249,9 @@ Key settings in more detail: - `access_control_allow_origin` — the origin a browser lets read pixa's responses, sent as the CORS `Access-Control-Allow-Origin` header: `*`, the - default, is any site; otherwise one origin, scheme and host only, such as - `https://example.com`. Anything else aborts startup + default, is any site; otherwise one origin: scheme, host and optional port, + exactly as the browser sends it, such as `https://example.com`. Anything + else aborts startup - `allowlist_hosts` — list of allowed upstream hosts - `blocked_networks` — list of CIDR ranges to refuse for SSRF protection, added to the always-enforced built-in ranges (loopback, private, diff --git a/TODO.md b/TODO.md index 848a58e..b619ac9 100644 --- a/TODO.md +++ b/TODO.md @@ -65,9 +65,9 @@ exhaustion server's write timeout and the per-request timeout); each has a `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`. + and optional port, exactly as the browser sends it; 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 e08a8d1..562e3a5 100644 --- a/config.example.yml +++ b/config.example.yml @@ -84,7 +84,8 @@ downstream_timeout: 60s # The origin a browser lets read pixa's responses, sent as the CORS # Access-Control-Allow-Origin header: "*" (the default) is any site; -# otherwise one origin, scheme and host only, such as https://example.com +# otherwise one origin: scheme, host and optional port, exactly as the +# browser sends it, such as https://example.com access_control_allow_origin: "*" # Maximum disk cache size in bytes. Explicit values are used exactly as diff --git a/internal/config/config.go b/internal/config/config.go index 1c17369..5c9504f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -624,13 +624,9 @@ func (c *Config) validateUpstreamMaxResponseSize() error { return nil } -// validateAccessControlAllowOrigin checks that access_control_allow_origin -// 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. +// validateAccessControlAllowOrigin accepts "*" or an origin exactly as a browser +// sends it: http or https, an IP address as netip writes it or a lowercase name +// with a letter in its last part, and an optional port 1-65535, not the default. func (c *Config) validateAccessControlAllowOrigin() error { origin := c.AccessControlAllowOrigin if origin == "*" { @@ -640,50 +636,54 @@ func (c *Config) validateAccessControlAllowOrigin() error { 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.Scheme+"://"+parsed.Host != origin || - strings.HasSuffix(origin, ":") { + if err != nil { return errOrigin } + defaultPort := map[string]string{"http": "80", "https": "443"}[parsed.Scheme] + if defaultPort == "" { + return errOrigin + } + + const letters = "abcdefghijklmnopqrstuvwxyz" + host := parsed.Hostname() + lastPart := host[strings.LastIndex(host, ".")+1:] - _, err = netip.ParseAddr(host) - if err != nil && !isHostName(host) { + addr, err := netip.ParseAddr(host) + + switch { + case err == nil && addr.Is6(): + host = "[" + addr.String() + "]" + case err == nil: + host = addr.String() + case strings.Trim(host, letters+"0123456789-.") != "": // a character other than these + return errOrigin + case !strings.ContainsAny(lastPart, letters): 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 { + // The value must be exactly the origin rebuilt from its parts. + rebuilt := parsed.Scheme + "://" + host + + port := parsed.Port() + if port != "" { + _, err := strconv.ParseUint(port, 10, 16) + if err != nil || port[0] == '0' || port == defaultPort { return errOrigin } + + rebuilt += ":" + port + } + + if rebuilt != origin { + 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