diff --git a/TODO.md b/TODO.md index 1fcd110..8fbb30d 100644 --- a/TODO.md +++ b/TODO.md @@ -40,7 +40,10 @@ P2: security: per-origin rate limiting `%2F`, `%3B` or `%2C`, as `GenerateSignedURL` wrote them, kept the `%3F` from being read as `?`, and the request failed with 401. `GenerateSignedURL` now writes Go's own encoding of the path. The README has an example with a query - and its signature. + and its signature. The path traversal check no longer reads the source query, + and keeps a `%` that does not start a percent-escape as it is instead of + refusing the URL, so a source path or query holding `%25` is accepted, while + a double-encoded `..` next to such a `%` is still refused. - 2026-10-15 `max_concurrent_processing` defaults to half the CPUs pixa sees, rounded down, and at least 1 (closes #242): it defaulted to one image per CPU, and a container with no CPU limit sees every CPU of the host, so on a 48-CPU diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index fa8fd10..1794d1f 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -47,6 +47,9 @@ func ValidateDimension(name string, value int) error { var sizeFormatRegex = regexp.MustCompile( `^(\d+)x(\d+)(?:\.(\w+))?$|^(orig)(?:\.(\w+))?$`) +// percentEscapeRegex matches one percent-escape, such as "%2e" +var percentEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}`) + // ParsedURL contains the parsed components of an image proxy URL. type ParsedURL struct { // Host is the source origin host (e.g., "cdn.example.com") @@ -100,12 +103,6 @@ func ParseImageURL(urlPath string) (*ParsedURL, error) { // parseImageComponents parses //., or // // for JPEG XL. func parseImageComponents(remainder string) (*ParsedURL, error) { - // Check for path traversal before any other processing - err := checkPathTraversal(remainder) - if err != nil { - return nil, err - } - // Find the last path segment, which holds "size" or "size.format" lastSlash := strings.LastIndex(remainder, "/") if lastSlash == -1 { @@ -161,6 +158,13 @@ func parseImageComponents(remainder string) (*ParsedURL, error) { path = path[:qIndex] } + // The source query names no file on the upstream, so only the host and + // the source path are checked for path traversal + err = checkPathTraversal(host + path) + if err != nil { + return nil, err + } + // Ensure path starts with / if !strings.HasPrefix(path, "/") { path = "/" + path @@ -186,12 +190,7 @@ func checkPathTraversal(path string) error { // Decode multiple times to catch double-encoding decoded := path for range 3 { - newDecoded, err := url.PathUnescape(decoded) - if err != nil { - // Malformed encoding is suspicious - return ErrPathTraversal - } - + newDecoded := decodePercentEscapes(decoded) if newDecoded == decoded { break } @@ -232,6 +231,18 @@ func checkPathTraversal(path string) error { return nil } +// decodePercentEscapes decodes every percent-escape in s, such as %2e, and +// keeps a "%" that does not start one as it is, where url.PathUnescape would +// refuse the whole string. A source path holding %25, a literal "%", has such +// a "%" after one more decoding. +func decodePercentEscapes(s string) string { + return percentEscapeRegex.ReplaceAllStringFunc(s, func(escape string) string { + decoded, _ := url.PathUnescape(escape) + + return decoded + }) +} + // parseSizeFormat parses strings like "800x600.webp", "orig.png" or "800x600" func parseSizeFormat(s string) (Size, ImageFormat, error) { matches := sizeFormatRegex.FindStringSubmatch(s)