Refuse an image URL whose host is not a host name or IP address (closes #275)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
A ? written into the host segment as %3F reached the parser decoded, so cdn.example.com?x=1 passed the allowlist as cdn.example.com, fetched the host's root path with that query, and was cached under its own keys: another unsigned spelling of one upstream request. The host segment is now accepted only as a host name, an IPv4 address or a bracketed IPv6 address, with an optional port, and anything else is refused with 400. An IPv6 address without brackets is now refused instead of needing a signature; a host name outside ASCII is refused, its punycode accepted. Model: opus-5-5
This commit was merged in pull request #285.
This commit is contained in:
@@ -575,7 +575,11 @@ pixa lower-cases the host of a `/v1/image/` URL before anything else uses it, so
|
|||||||
`CDN.Example.com` is matched, fetched, cached and counted against
|
`CDN.Example.com` is matched, fetched, cached and counted against
|
||||||
`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has
|
`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has
|
||||||
a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature
|
a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature
|
||||||
even when the host is allowlisted. A host holding `@` is refused with 400.
|
even when the host is allowlisted. The host must be a host name or an IPv4
|
||||||
|
address (letters, digits, hyphens, underscores and dots) or an IPv6 address in
|
||||||
|
brackets, either one optionally followed by `:` and a port. Any other host, such
|
||||||
|
as one holding `@`, or a `?`, `#`, `%`, `\` or space written percent-encoded, is
|
||||||
|
refused with 400.
|
||||||
|
|
||||||
### Configuration
|
### Configuration
|
||||||
|
|
||||||
|
|||||||
@@ -30,6 +30,17 @@ P2: security: per-origin rate limiting
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-15 the host of a `/v1/image/` URL must be a host name or an IPv4
|
||||||
|
address (letters, digits, hyphens, underscores and dots) or an IPv6 address in
|
||||||
|
brackets, either one optionally followed by `:` and a port, and any other host
|
||||||
|
is refused with 400 (closes #275): only `@` was refused, so a `?` written as
|
||||||
|
`%3F` gave the host `cdn.example.com?x=1`, which Go's URL parser reads as
|
||||||
|
`cdn.example.com` with a query. Such a URL passed the allowlist, and each text
|
||||||
|
after the `?` fetched the host's root path and cached the answer under keys of
|
||||||
|
its own. A `#`, `%`, `\` or space written percent-encoded is refused the same
|
||||||
|
way, and so is an IPv6 address without brackets, which needed a signature
|
||||||
|
before. Underscores are kept because `allowlist_hosts` accepts host names
|
||||||
|
holding them.
|
||||||
- 2026-10-15 the README says how a source URL with a query is written into an
|
- 2026-10-15 the README says how a source URL with a query is written into an
|
||||||
image URL, and the image route accepts it (closes #255): the query follows the
|
image URL, and the image route accepts it (closes #255): the query follows the
|
||||||
source path after `%3F`. pixa now reads the image URL's path decoded once, so
|
source path after `%3F`. pixa now reads the image URL's path decoded once, so
|
||||||
|
|||||||
@@ -264,3 +264,79 @@ func TestHandleImage_AllowlistOrSignature(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestHandleImage_InvalidHost_Returns400 sends image URLs whose host no URL
|
||||||
|
// host can be, and checks that each is refused with 400 naming the host as
|
||||||
|
// decoded: allowlistedHost with a character after it that a URL host cannot
|
||||||
|
// hold, percent-encoded, an IPv6 address without brackets, and brackets
|
||||||
|
// around something that is not an IPv6 address. A "?" or "#" after
|
||||||
|
// allowlistedHost would otherwise end the host when the source URL is read,
|
||||||
|
// so the request would pass the allowlist and be fetched and cached under a
|
||||||
|
// host that is not the one fetched.
|
||||||
|
func TestHandleImage_InvalidHost_Returns400(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
photos := newPhotoFetcher(t, allowlistedHost)
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
writtenHost string
|
||||||
|
decodedHost string
|
||||||
|
}{
|
||||||
|
{"? written as %3F", allowlistedHost + "%3Fx=1", allowlistedHost + "?x=1"},
|
||||||
|
{"# written as %23", allowlistedHost + "%23x", allowlistedHost + "#x"},
|
||||||
|
{"% written as %25", allowlistedHost + "%253F", allowlistedHost + "%3F"},
|
||||||
|
{`\ written as %5C`, allowlistedHost + "%5Cx", allowlistedHost + `\x`},
|
||||||
|
{"space written as %20", allowlistedHost + "%20", allowlistedHost + " "},
|
||||||
|
{"IPv6 address without brackets", "2001:db8::1", "2001:db8::1"},
|
||||||
|
{"IPv4 address in brackets", "[203.0.113.5]", "[203.0.113.5]"},
|
||||||
|
{"no address in brackets", "[abc]", "[abc]"},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
rec := sendGet(t, newImageRoute(t, photos), photoURL(tt.writtenHost))
|
||||||
|
checkErrorBody(t, rec, http.StatusBadRequest,
|
||||||
|
fmt.Sprintf("invalid image URL: invalid source host: %q",
|
||||||
|
tt.decodedHost))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleImage_HostForms_Served checks that the image route serves, with
|
||||||
|
// a valid signature, the image at photoPath on a host name, an IPv4 address,
|
||||||
|
// an IPv6 address in brackets, and a host followed by ":" with or without a
|
||||||
|
// port.
|
||||||
|
func TestHandleImage_HostForms_Served(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
hosts := []string{
|
||||||
|
allowlistedHost,
|
||||||
|
"img_1.example.com",
|
||||||
|
"203.0.113.5",
|
||||||
|
"[2001:db8::1]",
|
||||||
|
allowlistedHost + ":8443",
|
||||||
|
"[2001:db8::1]:8443",
|
||||||
|
allowlistedHost + ":",
|
||||||
|
}
|
||||||
|
|
||||||
|
photos := newPhotoFetcher(t, hosts...)
|
||||||
|
exp := time.Now().Add(time.Hour)
|
||||||
|
|
||||||
|
for _, host := range hosts {
|
||||||
|
t.Run(host, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
sig := photoSignature(testSigningKey, host, exp)
|
||||||
|
|
||||||
|
rec := sendGet(t, newImageRoute(t, photos),
|
||||||
|
photoURLWithSig(host, sig, exp))
|
||||||
|
if rec.Code != http.StatusOK {
|
||||||
|
t.Errorf("status = %d, want %d: %s",
|
||||||
|
rec.Code, http.StatusOK, rec.Body)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package imgcache
|
|||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"net/netip"
|
||||||
"net/url"
|
"net/url"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strconv"
|
"strconv"
|
||||||
@@ -50,6 +51,14 @@ var sizeFormatRegex = regexp.MustCompile(
|
|||||||
// percentEscapeRegex matches one percent-escape, such as "%2e"
|
// percentEscapeRegex matches one percent-escape, such as "%2e"
|
||||||
var percentEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}`)
|
var percentEscapeRegex = 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
|
||||||
|
// in brackets (hex digits, ":" and "."), either one optionally followed by ":"
|
||||||
|
// and a port, which may be empty. Its one group holds the text in brackets,
|
||||||
|
// which parseImageComponents also checks is an IPv6 address.
|
||||||
|
var hostRegex = regexp.MustCompile(
|
||||||
|
`^(?:[A-Za-z0-9_.-]+|\[([0-9A-Fa-f:.]+)\])(?::[0-9]*)?$`)
|
||||||
|
|
||||||
// ParsedURL contains the parsed components of an image proxy URL.
|
// ParsedURL contains the parsed components of an image proxy URL.
|
||||||
type ParsedURL struct {
|
type ParsedURL struct {
|
||||||
// Host is the source origin host (e.g., "cdn.example.com")
|
// Host is the source origin host (e.g., "cdn.example.com")
|
||||||
@@ -141,10 +150,20 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
|
|||||||
return nil, ErrMissingHost
|
return nil, ErrMissingHost
|
||||||
}
|
}
|
||||||
|
|
||||||
// A URL reads what comes before "@" as a user name and password, and the
|
// Only the forms hostRegex describes are accepted. A "?", "#" or "@" in
|
||||||
// rest as the host
|
// the host would make Go's URL parser, which the allowlist check and the
|
||||||
if strings.Contains(host, "@") {
|
// fetcher use, read a different host from the one allowlisted and cached
|
||||||
return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host)
|
hostMatch := hostRegex.FindStringSubmatch(host)
|
||||||
|
if hostMatch == nil {
|
||||||
|
return nil, fmt.Errorf("%w: %q", ErrInvalidHost, host)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Go's URL parser refuses brackets around anything but an IPv6 address
|
||||||
|
if inBrackets := hostMatch[1]; inBrackets != "" {
|
||||||
|
addr, err := netip.ParseAddr(inBrackets)
|
||||||
|
if err != nil || !addr.Is6() {
|
||||||
|
return nil, fmt.Errorf("%w: %q", ErrInvalidHost, host)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Host names do not depend on letter case, so the allowlist, the upstream
|
// Host names do not depend on letter case, so the allowlist, the upstream
|
||||||
|
|||||||
Reference in New Issue
Block a user