From 729c0ff222cb460487fb0652e3b7c0ccfad78eb1 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 01:02:08 +0000 Subject: [PATCH] Encode ' in the source query; the generator compares the path as written A "'" in the source query is percent-encoded with the other characters a browser encodes in a query. The generator page compares the URL pixa would fetch with the path as written, which url.URL keeps in RawPath, so a path holding %2F is refused whatever else it holds. It also refuses a source URL holding a user name and password, which pixa does not send, and a source path holding a ".." segment, as /v1/image/ does. README.md and TODO.md say so. Model: opus-5-5 --- README.md | 22 ++++++++-------- TODO.md | 24 ++++++++++-------- internal/handlers/auth.go | 46 ++++++++++++++++++++++++++++------ internal/imgcache/urlparser.go | 9 ++++--- 4 files changed, 69 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index d6f3a3a..a8a9242 100644 --- a/README.md +++ b/README.md @@ -339,11 +339,11 @@ percent-escape. So `my%20cat.jpg` in the image URL is the source path `/my%20cat.jpg`, fetched as `/my%2520cat.jpg`; and `100%25.png` is fetched as `/100%25.png`. The source query is the source URL's query as that URL writes it, its percent-escapes included, and is sent as it is, except that the characters a -browser percent-encodes in a query (a space, `"`, `<`, `>`, control characters -and characters outside ASCII) and a `%` that does not begin a percent-escape are -percent-encoded. pixa signs and caches the query in that form, so -`img%3Fname=my%20cat` and `img%3Fname=my%2520cat` in the image URL are the same -source query, `name=my%20cat`. +browser percent-encodes in a query (a space, `"`, `'`, `<`, `>`, control +characters and characters outside ASCII) and a `%` that does not begin a +percent-escape are percent-encoded. pixa signs and caches the query in that +form, so `img%3Fname=my%20cat` and `img%3Fname=my%2520cat` in the image URL are +the same source query, `name=my%20cat`. A size with one side `0` has that side scaled to keep the source's proportions, and is refused with 400 when that side would come out above 8192, the most @@ -459,11 +459,13 @@ nor change what it asks for. sent with an empty format, or none, also makes a JPEG XL URL. Width and height both empty or `0` keep the original size; if only one of them is empty or `0`, that side is scaled to keep the image's proportions. A URL whose - scaled side would come out above 8192 answers 400. So does a source URL that - the image URL would fetch written another way, such as one whose path holds - `%2F`, fetched as `/`, or `(`, fetched as `%28`, as pixa keeps the source - path decoded and fetches it percent-encoded again (see Routes); the page then - shows the URL pixa would fetch. + scaled side would come out above 8192 answers 400, and so does a source URL + whose path holds a `..` segment. So does a source URL that the image URL + would fetch written another way, and the page then shows the URL pixa would + fetch: one with a user name and password, which pixa does not send, or one + whose path holds `%2F`, fetched as `/`, or `(`, fetched as `%28`, as pixa + keeps the source path decoded and fetches it percent-encoded again (see + Routes). 3. The page shows the URL, `https:///v1/e//img.`, and when it expires. `` is the host the page was opened on, and the URL starts with `http` instead while `debug` is on. The name after the token is ignored diff --git a/TODO.md b/TODO.md index 79c5fdb..4c56de7 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,19 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-16 the source URL a request fetches is built with `net/url` from the + source host, path and query, not joined as text (closes #288). The source + path, decoded, is fetched percent-encoded again, so `100%25.png` fetches + `/100%25.png` where it answered 401, and `my%20cat.png` and `my%2520cat.png` + fetch different paths where both fetched `/my%20cat.png` under two cache keys. + The source query stays as the source URL writes it; the characters a browser + percent-encodes in a query, such as `'` or a space, which was sent raw and + answered 502, are percent-encoded when the image URL is read, so the query is + signed and cached as it is sent. The generator page answers 400, showing the + URL pixa would fetch, for a source URL its image URL would fetch written + another way, such as with `%2F` in the path, whatever else the path holds, or + with a user name and password. It also answers 400 for a source path holding a + `..` segment, as `/v1/image/` refuses one. - 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips build a huge image (closes #282): libvips' thumbnail scaled the whole source to cover the requested size and copied that into memory before it cut out the @@ -42,17 +55,6 @@ P2: security: per-origin rate limiting resident memory peaked at about 242 MiB for the 8192x64 source and 431 MiB for the 8192x1 one. The output can sit a fraction of an output pixel from where it was. -- 2026-10-16 the source URL a request fetches is built with `net/url` from the - source host, path and query, not joined as text (closes #288). The source - path, decoded, is fetched percent-encoded again, so `100%25.png` fetches - `/100%25.png` where it answered 401, and `my%20cat.png` and `my%2520cat.png` - fetch different paths where both fetched `/my%20cat.png` under two cache keys. - The source query stays as the source URL writes it; the characters a browser - percent-encodes in a query, such as a space, which was sent raw and answered - 502, are percent-encoded when the image URL is read, so the query is signed - and cached as it is sent. The generator page answers 400, showing the URL pixa - would fetch, for a source URL its image URL would fetch written another way, - such as with `%2F` in the path. - 2026-10-15 a `/v1/image/` URL whose source path or source query holds a `#`, written as `%23`, is refused with 400 (closes #286): the source URL built from it ended at the `#` when fetched, so each text after it fetched the same diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 173f2c7..d51b569 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -9,7 +9,9 @@ import ( "math" "net/http" "net/url" + "slices" "strconv" + "strings" "time" "sneak.berlin/go/pixa/internal/clientip" @@ -133,6 +135,16 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { return } + // /v1/image/ refuses a ".." segment in the source path, and so does + // the generator. The path is decoded, so %2e%2e counts too + if slices.Contains(strings.Split(parsed.Path, "/"), "..") { + s.renderGeneratorWithForm(w, r, + "Source URL path cannot hold a .. segment", r.Form, + http.StatusBadRequest) + + return + } + payload, expiresAt, ttl, err := buildGeneratePayload(parsed, r.Form) if err != nil { s.renderGeneratorWithForm(w, r, err.Error(), r.Form, @@ -141,17 +153,14 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { return } - // The image URL must fetch the source URL as it was written. The - // token keeps the path decoded and pixa fetches it percent-encoded - // again, so a path written another way, such as with %2F, would fetch - // another URL. The scheme is not kept: only path and query are compared + // The image URL must fetch the source URL as it was written sourceRequest := payload.ToImageRequest() sourceRequest.AllowHTTP = s.config.AllowHTTP + fetchURL := sourceRequest.SourceURL() - fetchURL, err := url.Parse(sourceRequest.SourceURL()) - if err != nil || fetchURL.RequestURI() != parsed.RequestURI() { + if !fetchedAsWritten(parsed, fetchURL) { s.renderGeneratorWithForm(w, r, "Source URL cannot be fetched as "+ - "written: pixa would fetch "+sourceRequest.SourceURL(), + "written: pixa would fetch "+fetchURL, r.Form, http.StatusBadRequest) return @@ -189,6 +198,29 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { } } +// fetchedAsWritten reports whether fetchURL, the URL pixa fetches for an +// image URL made from the source URL source, is source as it was written, +// apart from the scheme, which the token does not keep. It is not when source +// holds a user name and password, which the token does not keep either, or a +// "?" with no query after it. The token keeps the source path decoded and +// pixa fetches it percent-encoded again, so a path written another way, such +// as with %2F, is fetched as another path. url.URL keeps the path as written +// in RawPath when it is not the decoded path percent-encoded again. +// EscapedPath ignores RawPath when it also holds a character net/url encodes, +// such as a space, so RawPath is compared itself. +func fetchedAsWritten(source *url.URL, fetchURL string) bool { + writtenPath := source.RawPath + if writtenPath == "" { + writtenPath = source.EscapedPath() + } + + fetched, err := url.Parse(fetchURL) + + return err == nil && source.User == nil && !source.ForceQuery && + fetched.EscapedPath() == writtenPath && + fetched.RawQuery == source.RawQuery +} + // buildGeneratePayload parses the numeric form fields and assembles the // 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 diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index edcd344..6c58c20 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -52,10 +52,11 @@ var sizeFormatRegex = regexp.MustCompile( var percentEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}`) // queryEscapeRegex matches a percent-escape, or one character that a browser -// percent-encodes in a URL's query: one outside the printable ASCII characters -// "!" to "~", such as a space, a control character or a character outside -// ASCII, or one of `"#<>`. A "%" that does not begin a percent-escape matches too. -var queryEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}|[^!-~]|["#<>%]`) +// percent-encodes in the query of an http or https URL: one outside the +// printable ASCII characters "!" to "~", such as a space, a control character +// or a character outside ASCII, or one of `"#'<>`. A "%" that does not begin a +// percent-escape matches too. +var queryEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}|[^!-~]|["#'<>%]`) // hostRegex matches the forms the host of an image URL may take: a host name // or an IPv4 address (letters, digits, "-", "_" and "."), or an IPv6 address