diff --git a/internal/validators/endpoint.go b/internal/validators/endpoint.go index 037aeac0..329fe01a 100644 --- a/internal/validators/endpoint.go +++ b/internal/validators/endpoint.go @@ -65,8 +65,20 @@ func isPrivateIP(ip net.IP) bool { {parseCIDR("192.0.2.0/24")}, // TEST-NET-1 {parseCIDR("198.51.100.0/24")}, // TEST-NET-2 {parseCIDR("203.0.113.0/24")}, // TEST-NET-3 - {parseCIDR("224.0.0.0/4")}, // multicast - {parseCIDR("240.0.0.0/4")}, // reserved + // The two below sat in the same IANA special-purpose registry as every + // range above and were the only members of it still reachable. All three + // TEST-NETs were blocked while these were not, which was an oversight + // rather than a decision. + // + // 198.18.0.0/15 is the one that mattered: it is not globally routable, so + // organisations use it internally EXACTLY BECAUSE it looks public — as a + // lab range, or to number a network they do not want colliding with RFC + // 1918. An SSRF there reaches a real internal service, and reaches it via + // an address every "is this private?" intuition says is fine. + {parseCIDR("198.18.0.0/15")}, // RFC 2544 benchmarking + {parseCIDR("192.88.99.0/24")}, // 6to4 relay anycast (deprecated, RFC 7526) + {parseCIDR("224.0.0.0/4")}, // multicast + {parseCIDR("240.0.0.0/4")}, // reserved // IPv6 transition mechanisms EMBED an arbitrary IPv4 address inside an // IPv6 one, so a v6 literal in these ranges reaches a v4 destination // that every check above would have refused — 2002:7f00:0001:: is diff --git a/internal/validators/safe_http_test.go b/internal/validators/safe_http_test.go index b906fdc0..ea0b4fa6 100644 --- a/internal/validators/safe_http_test.go +++ b/internal/validators/safe_http_test.go @@ -5,6 +5,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -44,3 +45,57 @@ func TestSafeHTTPClientAllowPrivate_AllowsPrivateIP(t *testing.T) { require.NoError(t, err) assert.Equal(t, "ok", string(body)) } + +// TestSafeHTTPClient_RejectsEverySpecialPurposeRange walks the IANA +// special-purpose ranges as a table, so the block list is asserted as a SET +// rather than through whichever range a given test happened to pick. +// +// It exists because two of these were missing and nothing noticed: all three +// TEST-NETs were blocked while 198.18.0.0/15 and 192.88.99.0/24 — from the same +// registry, equally non-routable — were reachable. A per-range table makes the +// next omission visible as a missing row instead of an absence of evidence. +// +// 198.18.0.0/15 is the one with real exposure. Not being globally routable is +// exactly why organisations use it internally — as a lab range, or to number a +// network they do not want colliding with RFC 1918 — so an SSRF there reaches a +// live internal service via an address that reads as public. +func TestSafeHTTPClient_RejectsEverySpecialPurposeRange(t *testing.T) { + cases := []struct{ ip, why string }{ + {"10.1.2.3", "RFC 1918"}, + {"172.16.5.5", "RFC 1918"}, + {"192.168.1.1", "RFC 1918"}, + {"127.0.0.1", "loopback"}, + {"169.254.169.254", "link-local / cloud metadata"}, + {"100.64.1.1", "CGNAT"}, + {"0.0.0.5", "this network"}, + {"192.0.0.5", "IETF protocol assignments"}, + {"192.0.2.5", "TEST-NET-1"}, + {"198.51.100.5", "TEST-NET-2"}, + {"203.0.113.5", "TEST-NET-3"}, + {"198.18.0.5", "RFC 2544 benchmarking"}, + {"198.19.255.5", "RFC 2544 benchmarking, upper half of the /15"}, + {"192.88.99.5", "6to4 relay anycast (RFC 7526)"}, + {"224.0.0.5", "multicast"}, + {"240.0.0.5", "reserved"}, + {"::1", "IPv6 loopback"}, + {"fc00::5", "IPv6 ULA"}, + {"fe80::5", "IPv6 link-local"}, + } + for _, tc := range cases { + t.Run(tc.ip, func(t *testing.T) { + _, err := SafeHTTPClient(context.Background(), "https://"+hostFor(tc.ip)+"/x", time.Second) + require.Error(t, err, "%s (%s) must be refused", tc.ip, tc.why) + assert.Contains(t, err.Error(), "private/internal networks are not allowed", + "%s (%s) must be refused by the range check, not by a DNS or TLS failure "+ + "that would mask a missing range", tc.ip, tc.why) + }) + } +} + +// hostFor brackets IPv6 literals for use in a URL authority. +func hostFor(ip string) string { + if strings.Contains(ip, ":") { + return "[" + ip + "]" + } + return ip +}