From 5b387675c44e83143db36f7a1b0a5671ca9aa492 Mon Sep 17 00:00:00 2001 From: Dimitri Herzog Date: Mon, 8 Jun 2026 10:37:59 +0200 Subject: [PATCH] test: add fuzz tests for untrusted-input parsers (#2092) --- Makefile | 18 +++++++- config/upstream_fuzz_test.go | 58 +++++++++++++++++++++++++ lists/parsers/hosts_funcs_test.go | 71 +++++++++++++++++++++++++++++++ util/arpa_fuzz_test.go | 65 ++++++++++++++++++++++++++++ 4 files changed, 211 insertions(+), 1 deletion(-) create mode 100644 config/upstream_fuzz_test.go create mode 100644 util/arpa_fuzz_test.go diff --git a/Makefile b/Makefile index e87ab7a7..ca166329 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: all clean generate generate-check build test e2e-test e2e-test-coverage lint run fmt docker-build help check-tools +.PHONY: all clean generate generate-check build test fuzz e2e-test e2e-test-coverage lint run fmt docker-build help check-tools .DEFAULT_GOAL:=help VERSION?=$(shell git describe --always --tags) @@ -27,6 +27,13 @@ GOLANG_LINT_VERSION=v2.12.2 GINKGO_PROCS?= +# Fuzzing. Fuzz target seed corpora run as ordinary tests on every `make test`; +# this is the opt-in discovery mode that actively generates new inputs. `go test +# -fuzz` only fuzzes one target in one package per invocation, so `make fuzz` +# loops over every Fuzz* target in FUZZ_PKGS, time-boxing each at FUZZ_TIME. +FUZZ_TIME?=30s +FUZZ_PKGS?=./config ./util ./lists/parsers + # Parallelism for e2e tests. e2e specs are dominated by container startup and # health-check waits rather than CPU, so oversubscribing beyond the core count # improves wall-clock time. Defaults to -p (one process per core) for local @@ -87,6 +94,15 @@ test: check-go ## run tests go tool ginkgo --label-filter="!e2e" --coverprofile=coverage.txt --covermode=atomic --cover -r ${GINKGO_PROCS} go tool cover -html coverage.txt -o coverage.html +fuzz: check-go ## run each fuzz target for FUZZ_TIME (default 30s); e.g. make fuzz FUZZ_TIME=2m + @set -e; \ + for pkg in $(FUZZ_PKGS); do \ + for target in $$(go test -list '^Fuzz' $$pkg | grep '^Fuzz'); do \ + echo "==> fuzzing $$target in $$pkg for $(FUZZ_TIME)"; \ + go test -run '^$$' -fuzz "^$$target$$" -fuzztime $(FUZZ_TIME) $$pkg; \ + done; \ + done + e2e-test: check-go check-docker ## run e2e tests docker buildx build \ --build-arg VERSION=blocky-e2e \ diff --git a/config/upstream_fuzz_test.go b/config/upstream_fuzz_test.go new file mode 100644 index 00000000..bcc79851 --- /dev/null +++ b/config/upstream_fuzz_test.go @@ -0,0 +1,58 @@ +package config + +import "testing" + +// FuzzParseUpstream exercises ParseUpstream with arbitrary input. The function +// parses untrusted upstream specifications, including DNS stamps (sdns://...) +// whose payload is base64-decoded and then sliced by an attacker-controlled +// length byte in stampWithoutAddrPort. The core invariant is therefore that it +// must never panic, no matter how malformed the input is. +// +// For a successfully parsed *non-stamp* upstream it additionally asserts that +// String() is a parse fixpoint: re-parsing u.String() succeeds and yields the +// same String(). Stamps are exempt because String() intentionally omits stamp +// metadata (certificate fingerprints, bootstrap IPs, common name), so a +// round-trip through it is lossy by design. +func FuzzParseUpstream(f *testing.F) { + seeds := []string{ + "", ":53", "udp", + "8.8.8.8:5353", + "[2001:4860:4860::8888]:5353", + "tcp-tls:dns.example.com:853", + "https://dns.example.com:8443/dns-query", + "quic://dns.adguard.com", + "dns.example.com#cloudflare-dns.com", + // DNS stamps, including a deliberately malformed one (invalid..hostname). + "sdns://", + "sdns://AAcAAAAAAAAABzguOC44Ljg", + "sdns://AgcAAAAAAAAABzEuMC4wLjEAEWludmFsaWQuLmhvc3RuYW1lCi9kbnMtcXVlcnk", + "sdns://AwAAAAAAAAAADTE0OS4xMTIuMTEyLjkgIINqrLwxXg3E7t8E8DTYfvzaJI-U3WvkQgHQj8JBJgkJcXVhZDkubmV0", + } + for _, s := range seeds { + f.Add(s) + } + + f.Fuzz(func(t *testing.T, in string) { + u, err := ParseUpstream(in) + if err != nil { + return // rejecting input is fine; we only require that it not panic + } + + // String() is lossy for stamps by design, so the fixpoint check below + // does not apply to them. + if isDNSStamp(in) { + return + } + + s1 := u.String() + + u2, err := ParseUpstream(s1) + if err != nil { + t.Fatalf("ParseUpstream(%q) succeeded but re-parsing its String() %q failed: %v", in, s1, err) + } + + if s2 := u2.String(); s2 != s1 { + t.Fatalf("String() is not a fixpoint for input %q: first %q, second %q", in, s1, s2) + } + }) +} diff --git a/lists/parsers/hosts_funcs_test.go b/lists/parsers/hosts_funcs_test.go index 73fc2f05..14244c21 100644 --- a/lists/parsers/hosts_funcs_test.go +++ b/lists/parsers/hosts_funcs_test.go @@ -173,3 +173,74 @@ func FuzzMightBeIP(f *testing.F) { } }) } + +// --- entry parsing (HostListEntry / HostsFileEntry / HostsIterator) --- + +// FuzzHostsUnmarshalText feeds arbitrary input to the hosts-list parsers, which +// consume untrusted downloaded block/allow lists line by line. Neither the +// individual entry types nor the HostsIterator that dispatches between them may +// panic on any input. +// +// Beyond the no-panic guarantee it asserts the post-conditions the parsers +// document: +// - a successful HostsFileEntry has a non-nil IP and only valid host names; +// - HostListEntry normalization is idempotent: re-parsing its own output is a +// fixpoint, since a normalized entry is already in canonical form. +func FuzzHostsUnmarshalText(f *testing.F) { + for _, s := range []string{ + "example.com", + "||ads.example.com^", + "0.0.0.0 ads.example.com", + "127.0.0.1 localhost local.host", + "::1 ip6-localhost", + "fe80::1%eth0 router.local", + "*.tracker.example.com", + `/^ads\.example\.com$/`, + "münchen.example.de", + "xn--mnchen-3ya.example.de", + "", " ", "\t", "1.2.3.4", + } { + f.Add(s) + } + + f.Fuzz(func(t *testing.T, in string) { + data := []byte(in) + + // None of the parsers may panic, regardless of whether they accept the input. + var list HostListEntry + listErr := list.UnmarshalText(data) + + var file HostsFileEntry + fileErr := file.UnmarshalText(data) + + var iter HostsIterator + _ = iter.UnmarshalText(data) + + // A successful hosts-file entry must carry a usable IP and valid names. + if fileErr == nil { + if file.IP == nil { + t.Fatalf("HostsFileEntry parsed %q but IP is nil", in) + } + + _ = file.forEachHost(func(host string) error { + if err := validateDomainName(host); err != nil { + t.Fatalf("HostsFileEntry parsed %q but emitted host %q is invalid: %v", in, host, err) + } + + return nil + }) + } + + // Normalizing an already-normalized host-list entry must be a fixpoint. + if listErr == nil { + var reparsed HostListEntry + if err := reparsed.UnmarshalText([]byte(list.String())); err != nil { + t.Fatalf("HostListEntry %q normalized to %q which fails to re-parse: %v", in, list.String(), err) + } + + if reparsed != list { + t.Fatalf("HostListEntry normalization is not idempotent: %q -> %q -> %q", in, list.String(), reparsed.String()) + } + } + }) +} diff --git a/util/arpa_fuzz_test.go b/util/arpa_fuzz_test.go new file mode 100644 index 00000000..cf9c122b --- /dev/null +++ b/util/arpa_fuzz_test.go @@ -0,0 +1,65 @@ +package util + +import ( + "net" + "testing" + + "github.com/miekg/dns" +) + +// FuzzParseIPFromArpaAddr exercises ParseIPFromArpaAddr, which parses reverse-DNS +// (PTR) names taken directly off the network, with arbitrary input. However +// malformed the name is, parsing must never panic. +func FuzzParseIPFromArpaAddr(f *testing.F) { + for _, s := range []string{ + "", "in-addr.arpa.", "ip6.arpa.", + "8.8.8.8.in-addr.arpa.", + "1.0.0.127.in-addr.arpa.", + "x.8.8.8.in-addr.arpa.", + "256.8.8.8.in-addr.arpa.", + "4.3.2.1.0.d.d.4.1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa.", + } { + f.Add(s) + } + + f.Fuzz(func(t *testing.T, in string) { + _, _ = ParseIPFromArpaAddr(in) // must not panic + }) +} + +// FuzzParseIPFromArpaAddrRoundtrip cross-checks ParseIPFromArpaAddr against +// dns.ReverseAddr (from miekg/dns, already a dependency) as an oracle: for any +// valid IPv4 or IPv6 address, the arpa name produced by ReverseAddr must parse +// back to the same address. +func FuzzParseIPFromArpaAddrRoundtrip(f *testing.F) { + f.Add([]byte{8, 8, 8, 8}) + f.Add([]byte{127, 0, 0, 1}) + f.Add([]byte(net.ParseIP("2001:db8::1").To16())) + + f.Fuzz(func(t *testing.T, raw []byte) { + var ip net.IP + + switch len(raw) { + case net.IPv4len: + ip = net.IPv4(raw[0], raw[1], raw[2], raw[3]) + case net.IPv6len: + ip = net.IP(raw).To16() + default: + return // only 4- and 16-byte inputs map to a concrete IP + } + + arpa, err := dns.ReverseAddr(ip.String()) + if err != nil { + t.Fatalf("dns.ReverseAddr(%s) failed: %v", ip, err) + } + + got, err := ParseIPFromArpaAddr(arpa) + if err != nil { + t.Fatalf("ParseIPFromArpaAddr(%q) failed for ip %s: %v", arpa, ip, err) + } + + if !got.Equal(ip) { + t.Fatalf("round-trip mismatch: ip %s -> arpa %q -> %s", ip, arpa, got) + } + }) +}