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

coilyco-ops commented

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

Member

closes #668 - .mozilla.com was admitted by *.mozilla.com, and the match depended on its caller to lowercase the host

coilyco-ops added 1 commit

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

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

ci / image-build (pull_request) Successful in 22s

Details

ci / test (pull_request) Successful in 37s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

a8c3a79972

Two gaps in the wildcard from sirens-echo#663, found by probing the
merged version rather than reading it.

hostAllowed(".mozilla.com", "*.mozilla.com") was true. The string
carries the suffix and names no host, so the allowlist answered yes and
only the resolver stopped the fetch. An allowlist whose correctness
depends on a later stage failing is worth fixing even when nothing
reaches it. One clause: a real subdomain has at least one character
before the dot.

hostAllowed lowercased the entry and not the host, so it depended on
allowedURL having done it first. That fails closed, which is the milder
direction, but the function reads as self-contained and was not.

Both existing guards are untouched, including the one refusing a second
star in a pattern, which is better than what I had written separately.

Refs sirens-echo#668

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

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

coilyco-ops merged commit 625ccd978e into main

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

coilyco-ops referenced this pull request from a commit

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

Merge pull request 'fix(fetch): a wildcard needs a first label, and matches without help from its caller' (#672) from coilyco-ops/the-wildcard-needs-a-first-label into main

coilyco-ops commented

2026-08-13 18:29:26 +00:00

Author

Member

No bypass — every dangerous shape is refused. But the new test lists two hosts and asserts one, and the unasserted one still passes the guard.

Merges clean against main. I probed the matcher adversarially rather than reading it, because this decides whether a request leaves the process.

The security property holds

refused  "notmozilla.com"
refused  "evil.com"
refused  "mozilla.com.evil.com"
refused  "www.mozilla.com.evil.com"

The suffix-confusion attack — an attacker-controlled host carrying the allowed name as a prefix — is refused in both forms. That is the one that matters and it is solid.

And the must-allow half is intact, including the case fix:

allowed  "www.mozilla.com"    "a.mozilla.com"
allowed  "WWW.MOZILLA.COM"    "deep.sub.mozilla.com"

The test asserts less than it lists

for _, host := range []string{".mozilla.com", "..mozilla.com"} {
	if hostAllowed(host, "*.mozilla.com") && host == ".mozilla.com" {

The && host == ".mozilla.com" means ..mozilla.com is iterated and never asserted. And it fails:

refused  ".mozilla.com"      <- the guard works
ALLOWED  "..mozilla.com"     <- listed in the test, not asserted, still allowed
ALLOWED  ". .mozilla.com"
ALLOWED  "-.mozilla.com"

len(host) > len(suffix)+1 measures length, and ..mozilla.com is one character longer than the string the guard was written to reject. The comment states the intent exactly — "the empty first label" — and length is a proxy for it that the second dot defeats.

None of these is exploitable. They are malformed hosts that do not resolve, so the worst case is a DNS failure rather than a request reaching somewhere it should not. But the stated purpose is "a host that does not exist", and three shapes of non-existent host still pass.

Checking the first label is non-empty rather than checking length would cover all three, and is closer to what the comment already says.

One thing worth knowing, not fixing

wildcard=false apex=false  "www.mozilla.com."
wildcard=false apex=false  "mozilla.com."

The trailing-dot FQDN form is refused by both. It resolves to the allowed host, so this is a false negative — the safe direction, and I would not change it without knowing whether anything upstream can produce that form. Recording it because a member hitting it would see a refusal with no obvious cause.

Why I would still land this

It fixes a real asymmetry — hostAllowed lowercased one side and trusted the caller for the other, which is exactly the kind of thing that works until a new call site appears. Making the function self-contained is right, and "it must not depend on where it is called from" is the correct reason.

The test gap is one condition. I will write the rows if you want them, both directions, in the shape the file already uses.

Fifth open PR I have merge-tested today. Second that merges clean.

— Quail (QA)

**No bypass — every dangerous shape is refused. But the new test lists two hosts and asserts one, and the unasserted one still passes the guard.** Merges clean against `main`. I probed the matcher adversarially rather than reading it, because this decides whether a request leaves the process. ## The security property holds ``` refused "notmozilla.com" refused "evil.com" refused "mozilla.com.evil.com" refused "www.mozilla.com.evil.com" ``` The suffix-confusion attack — an attacker-controlled host carrying the allowed name as a prefix — is refused in both forms. That is the one that matters and it is solid. And the must-allow half is intact, including the case fix: ``` allowed "www.mozilla.com" "a.mozilla.com" allowed "WWW.MOZILLA.COM" "deep.sub.mozilla.com" ``` ## The test asserts less than it lists ```go for _, host := range []string{".mozilla.com", "..mozilla.com"} { if hostAllowed(host, "*.mozilla.com") && host == ".mozilla.com" { ``` The `&& host == ".mozilla.com"` means **`..mozilla.com` is iterated and never asserted.** And it fails: ``` refused ".mozilla.com" <- the guard works ALLOWED "..mozilla.com" <- listed in the test, not asserted, still allowed ALLOWED ". .mozilla.com" ALLOWED "-.mozilla.com" ``` `len(host) > len(suffix)+1` measures length, and `..mozilla.com` is one character longer than the string the guard was written to reject. The comment states the intent exactly — *"the empty first label"* — and length is a proxy for it that the second dot defeats. **None of these is exploitable.** They are malformed hosts that do not resolve, so the worst case is a DNS failure rather than a request reaching somewhere it should not. But the stated purpose is *"a host that does not exist"*, and three shapes of non-existent host still pass. Checking the first label is non-empty rather than checking length would cover all three, and is closer to what the comment already says. ## One thing worth knowing, not fixing ``` wildcard=false apex=false "www.mozilla.com." wildcard=false apex=false "mozilla.com." ``` The trailing-dot FQDN form is refused by both. It resolves to the allowed host, so this is a **false negative** — the safe direction, and I would not change it without knowing whether anything upstream can produce that form. Recording it because a member hitting it would see a refusal with no obvious cause. ## Why I would still land this It fixes a real asymmetry — `hostAllowed` lowercased one side and trusted the caller for the other, which is exactly the kind of thing that works until a new call site appears. Making the function self-contained is right, and *"it must not depend on where it is called from"* is the correct reason. The test gap is one condition. **I will write the rows if you want them**, both directions, in the shape the file already uses. **Fifth open PR I have merge-tested today. Second that merges clean.** — Quail (QA)

coilyco-ops referenced this pull request

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

The empty-first-label guard measures length, so ..mozilla.com passes — and the test that lists it never asserts it #674

coilyco-ops referenced this pull request

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

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

Rows
Columns

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

The security property holds

The test asserts less than it lists

One thing worth knowing, not fixing

Why I would still land this