The fetch wildcard admits a host with an empty first label, and depends on its caller to lowercase #668

New issue

Closed

opened 2026-08-13 18:23:09 +00:00 by coilyco-ops · 0 comments

coilyco-ops commented

2026-08-13 18:23:09 +00:00

Member

Filed by Angie (ENG, claude seat) after building #663 in parallel and finding it merged. Rather than argue for my version I probed the landed one, and it has two gaps. Neither is exploitable today. One of them fails open, which is why it is worth closing anyway.

Measured against `hostAllowed` on main

hostAllowed(".mozilla.com",    "*.mozilla.com") = true    <- fails open
hostAllowed("WWW.MOZILLA.COM", "*.mozilla.com") = false   <- fails closed
hostAllowed("www.mozilla.com", "*.mozilla.com") = true    <- correct

One: a host whose first label is empty is treated as a subdomain

return strings.HasSuffix(host, "."+suffix)

".mozilla.com" ends with ".mozilla.com", so it passes. url.Parse("https://.mozilla.com").Hostname() returns exactly that, so the allowlist says yes to a string that names no host.

Not reachable as a fetch today, because the dialer then fails to resolve it. So this is a latent hazard rather than a live one: the allowlist's answer is wrong, and it is only the resolver that stops it. An allowlist whose correctness depends on a later stage failing is the shape worth removing, not the shape worth keeping.

One clause: len(host) > len(suffix)+1. A real subdomain always has at least one character before the dot.

Two: the match depends on its caller for case

hostAllowed lowercases the entry and not the host. allowedURL happens to lowercase before calling, so production is correct.

This one fails closed — a caller that forgets refuses a valid host rather than admitting an invalid one — so it is the milder of the two. It is still worth fixing, because the function reads as self-contained and is not. A second caller is the kind of thing that gets added without re-reading the callee.

What I am not claiming

The landed version is good, and it is better than mine in one place I did not think of: it refuses a pattern containing a second *, so *.a*b.com is a typo rather than a silent never-match. That guard stays.

Acceptance

".mozilla.com" refused against *.mozilla.com; an uppercase host matched against a lowercase entry; the existing * and bare-*. guards unchanged. Each new row fails if its clause is removed.

**Filed by Angie (ENG, claude seat)** after building https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/663 in parallel and finding it merged. Rather than argue for my version I probed the landed one, and it has two gaps. Neither is exploitable today. One of them fails **open**, which is why it is worth closing anyway. ## Measured against `hostAllowed` on main ``` hostAllowed(".mozilla.com", "*.mozilla.com") = true <- fails open hostAllowed("WWW.MOZILLA.COM", "*.mozilla.com") = false <- fails closed hostAllowed("www.mozilla.com", "*.mozilla.com") = true <- correct ``` ## One: a host whose first label is empty is treated as a subdomain ```go return strings.HasSuffix(host, "."+suffix) ``` `".mozilla.com"` ends with `".mozilla.com"`, so it passes. `url.Parse("https://.mozilla.com").Hostname()` returns exactly that, so the allowlist says yes to a string that names no host. **Not reachable as a fetch today**, because the dialer then fails to resolve it. So this is a latent hazard rather than a live one: the allowlist's answer is wrong, and it is only the resolver that stops it. An allowlist whose correctness depends on a later stage failing is the shape worth removing, not the shape worth keeping. One clause: `len(host) > len(suffix)+1`. A real subdomain always has at least one character before the dot. ## Two: the match depends on its caller for case `hostAllowed` lowercases the **entry** and not the **host**. `allowedURL` happens to lowercase before calling, so production is correct. This one fails closed — a caller that forgets refuses a valid host rather than admitting an invalid one — so it is the milder of the two. It is still worth fixing, because **the function reads as self-contained and is not.** A second caller is the kind of thing that gets added without re-reading the callee. ## What I am not claiming The landed version is good, and it is better than mine in one place I did not think of: it refuses a pattern containing a second `*`, so `*.a*b.com` is a typo rather than a silent never-match. That guard stays. ## Acceptance `".mozilla.com"` refused against `*.mozilla.com`; an uppercase host matched against a lowercase entry; the existing `*` and bare-`*.` guards unchanged. Each new row fails if its clause is removed.

coilyco-ops referenced this issue from a pull request that will close it,

2026-08-13 18:25:21 +00:00

fix(fetch): a wildcard needs a first label, and matches without help from its caller #672

coilyco-ops closed this issue

2026-08-13 18:28:04 +00:00

coilyco-ops referenced this issue from a pull request that will close it,

2026-08-13 18:30:19 +00:00

fix(fetch): a wildcard needs a real first label, and normalises its own input #673

coilyco-ops referenced this issue from a pull request that will close it,

2026-08-13 18:36:17 +00:00

fix(fetch): every label before the suffix has to be real #680

coilyco-ops referenced this issue