From ceadf8b1d45c2278c5e6e66f9df01d78019a4c71 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sat, 15 Aug 2026 03:10:00 -0500 Subject: [PATCH] fix(node): close three host-class gaps in the public-URL gate (#339) is_public_http_url is the shared gate for every operator-supplied outbound URL: peer announce, webhook creation, and the boot-time peer prune. Three host forms got past it that the checks around them plainly meant to exclude. A name with no dot never matched the `.local` / `.internal` suffix rules, so `http://internal/` and `http://wpad/` were accepted and left to the resolver to complete from its search domain. The dotless rule sits on the branch where the host did not parse as an IP literal, which keeps bracketed IPv6 (also dotless) on the accepting path; the existing 6to4 and NAT64 accept cases pin that. fec0::/10 fell through the IPv6 arm because the link-local mask does not cover it: 0xfec0 & 0xffc0 is 0xfec0, not 0xfe80. RFC 3879 deprecated the range, which argues for rejecting it rather than ignoring it. Trailing root dots were stripped once, so `localhost..` reduced to `localhost.` and matched neither the equality nor the suffix check. Stripping to fixation closes that. Each of the three is covered by a test that reddens when its own production line is reverted, and the dotted forms stay as negative controls so the suffix checks remain proven. --- crates/gitlawb-node/src/api/peers.rs | 64 +++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/crates/gitlawb-node/src/api/peers.rs b/crates/gitlawb-node/src/api/peers.rs index 3934e7a3..f0954fba 100644 --- a/crates/gitlawb-node/src/api/peers.rs +++ b/crates/gitlawb-node/src/api/peers.rs @@ -118,11 +118,11 @@ pub fn is_public_http_url(raw: &str) -> bool { Some(h) => h.to_ascii_lowercase(), None => return false, }; - // Drop a single trailing dot (FQDN root): `localhost.` resolves the same as + // Drop trailing dots (FQDN root): `localhost.` resolves the same as // `localhost`, so normalize before the suffix/equality checks below. - if let Some(stripped) = host.strip_suffix('.') { - host = stripped.to_string(); - } + // Strip to fixation rather than once, or `localhost..` reduces to + // `localhost.` and matches neither the equality nor the suffix check. + host.truncate(host.trim_end_matches('.').len()); if host.is_empty() || host == "localhost" || host.ends_with(".local") @@ -167,8 +167,13 @@ pub fn is_public_http_url(raw: &str) -> bool { } std::net::IpAddr::V6(v6) => { let s = v6.segments(); - // fc00::/7 (unique-local) or fe80::/10 (link-local) - if (s[0] & 0xfe00) == 0xfc00 || (s[0] & 0xffc0) == 0xfe80 { + // fc00::/7 (unique-local), fe80::/10 (link-local), or fec0::/10 + // (site-local, deprecated by RFC 3879 and so never a legitimate + // peer address, but still routable on networks that kept it). + if (s[0] & 0xfe00) == 0xfc00 + || (s[0] & 0xffc0) == 0xfe80 + || (s[0] & 0xffc0) == 0xfec0 + { return false; } // Any NAT64 address (64:ff9b::/32) that is not the cleanly @@ -183,6 +188,13 @@ pub fn is_public_http_url(raw: &str) -> bool { } } } + } else if !host.contains('.') { + // A name with no dot at all. The `.local` / `.internal` rules above are + // suffix checks, so a single label slips both, and the resolver decides + // what it means by appending a search domain we cannot see from here. + // Only reachable when the host did not parse as an IP literal, which is + // what keeps bracketed IPv6 (also dotless) on the accepting path. + return false; } true } @@ -577,6 +589,46 @@ mod tests { } } + #[test] + fn rejects_dotless_single_label_hosts() { + // `.local` and `.internal` are suffix checks, so a name with no dot at + // all never matches one. A single-label name is completed by whatever + // search domain the resolver is configured with, which is not a + // destination we can reason about from the URL. + for bad in [ + "http://local/", + "http://internal/", + "http://intranet/", + "http://wpad/", + "http://metadata/", + "http://local./", + "http://internal.:7545", + ] { + assert!(!is_public_http_url(bad), "{bad:?} must be rejected"); + } + } + + #[test] + fn rejects_deprecated_site_local_v6() { + // fec0::/10, deprecated by RFC 3879. The link-local mask (0xffc0 == + // 0xfe80) does not cover it: 0xfec0 & 0xffc0 is 0xfec0. + for bad in ["http://[fec0::1]/", "http://[feff:ffff::1]:7545"] { + assert!(!is_public_http_url(bad), "{bad:?} must be rejected"); + } + } + + #[test] + fn rejects_repeated_trailing_root_dots() { + // Stripping a single root dot leaves `localhost.`, which matches + // neither the equality nor the suffix check. The subdomain form + // (`node.localhost..`) is not asserted here: `.localhost` is not yet a + // rejected suffix on this base, so that case belongs to whichever + // change adds it rather than to this one. + for bad in ["http://localhost../", "http://localhost...:7545"] { + assert!(!is_public_http_url(bad), "{bad:?} must be rejected"); + } + } + #[test] fn accepts_6to4_and_nat64_wrapping_public_v4() { // Fold-and-recheck stays consistent with the mapped/compatible handling: