GHSA-9ppr-wfw9-xvr2: DNS Cache TTL Manipulation Failure in Netfoil via Go Value-Copy Range Iteration

netfoil (github.com/tinfoil-factory/netfoil) is a minimal, filtering DNS proxy and forwarding resolver implemented in Go. The software intercepts incoming DNS queries from local or network clients, evaluates the requested domain names against configured filtering rules, and forwards permitted queries to upstream recursive or authoritative resolvers. To reduce latency and minimize redundant upstream network traffic, netfoil maintains an in-memory cache of resolved DNS responses encapsulated within timedResponse structures.

In accordance with RFC 1035 and standard DNS caching architecture, a caching proxy must monotonically decrease the Time-To-Live (TTL) field of cached resource records by the number of seconds the record has resided in the cache before serving it to downstream clients. Furthermore, netfoil provides a configuration parameter (MaxTTL) designed to clamp excessively long upstream TTLs to a locally enforced maximum threshold.

In netfoil versions starting from 0.3.0 and prior to 0.6.1, both the TTL decrement mechanism and the MaxTTL clamping routine fail to modify the actual response packets stored in memory or served to clients. Tracked as GHSA-9ppr-wfw9-xvr2 and classified under CWE-563 (Assignment to Variable without Use) and CWE-684 (Incorrect Provision of Specified Functionality), this defect causes the proxy to emit static, un-decremented TTLs on cached responses and store uncapped TTLs in its internal cache.

The root cause of GHSA-9ppr-wfw9-xvr2 is a Go value-copy range iteration bug within internal/dns/dns_proxy.go. In Go, slices are reference-like descriptors pointing to an underlying contiguous array of elements. When iterating over a slice of non-pointer structs using the two-variable for _, element := range slice syntax, the Go runtime allocates a single local variable element and overwrites it with a shallow copy of each struct from the backing array on every iteration.

Because element is an independent stack-allocated value copy rather than a pointer or reference to the element inside the slice's backing array, any assignment to element.Field mutates only the temporary loop variable. Once the current loop iteration concludes, the modified copy is overwritten by the next element or discarded when the loop terminates, leaving the original slice completely unmodified.

This exact pattern occurs in two distinct locations within internal/dns/dns_proxy.go:

1. Cache Expiration and TTL Decrement (timedResponse.rewriteTTLs): When a cached DNS response is retrieved to answer a query, rewriteTTLs() clones the cached Answers slice (slices.Clone(t.response.Answers)) and calculates diffSeconds, the elapsed time since the response was cached. It then iterates over result.Answers using for _, a := range result.Answers. Although the loop correctly evaluates whether a.TTL >= diffSeconds and sets ok = false when a record has expired, the assignments a.TTL = a.TTL - diffSeconds and a.TTL = 0 only alter the local variable a. As long as ok remains true (meaning diffSeconds has not yet exceeded the initial TTL), the function returns result containing the original, unmodified TTL values.

2. Maximum TTL Enforcement (worker.process): When processing a fresh upstream DNS response prior to cache insertion, worker.process() checks if the response contains between 1 and 999 answers. It then executes for _, answer := range candidateResponse.Answers to compare answer.TTL against w.config.MaxTTL. When an upstream record exceeds MaxTTL, the assignment answer.TTL = w.config.MaxTTL mutates the ephemeral answer copy. Consequently, candidateResponse is stored in the cache with its original upstream TTL intact.

The defect and its corresponding fix are contained within commit ea42977900c864b7b1c42f26c5870c86008118a0 in internal/dns/dns_proxy.go. Examining the pre-patch implementation demonstrates how the value copies prevented state updates in both rewriteTTLs() and worker.process().

Vulnerable Implementation (Pre-v0.6.1)

func (t *timedResponse) rewriteTTLs() (result *Response, stillValid bool) {
	// ...
	result = &Response{
		// ...
		Answers: slices.Clone(t.response.Answers),
	}
 
	// BUG: 'a' is a value copy of the struct at each index of result.Answers
	for _, a := range result.Answers {
		if a.TTL >= diffSeconds {
			a.TTL = a.TTL - diffSeconds // Dead store: mutates copy 'a', not result.Answers
		} else {
			ok = false
			a.TTL = 0 // Dead store: mutates copy 'a', not result.Answers
		}
	}
	// ...
}

Similarly, inside worker.process():

if len(candidateResponse.Answers) > 0 && len(candidateResponse.Answers) < 1000 {
	// BUG: 'answer' is a value copy of each element in candidateResponse.Answers
	for _, answer := range candidateResponse.Answers {
		if answer.TTL > w.config.MaxTTL {
			answer.TTL = w.config.MaxTTL // Dead store: fails to clamp TTL in backing array
		}
	}
}

Patched Implementation (v0.6.1)

Commit ea42977900c864b7b1c42f26c5870c86008118a0 (merged via Pull Request #44) resolves both instances by switching to index-based iteration (for i := range ...) and indexing directly into the slice (result.Answers[i].TTL and candidateResponse.Answers[i].TTL).

diff --git a/internal/dns/dns_proxy.go b/internal/dns/dns_proxy.go
index 7b14f16..0ed3a1d 100644
--- a/internal/dns/dns_proxy.go
+++ b/internal/dns/dns_proxy.go
@@ -76,12 +76,12 @@ func (t *timedResponse) rewriteTTLs() (result *Response, stillValid bool) {
 		Answers:   slices.Clone(t.response.Answers),
 	}
 
-	for _, a := range result.Answers {
-		if a.TTL >= diffSeconds {
-			a.TTL = a.TTL - diffSeconds
+	for i := range result.Answers {
+		if result.Answers[i].TTL >= diffSeconds {
+			result.Answers[i].TTL = result.Answers[i].TTL - diffSeconds
 		} else {
 			ok = false
-			a.TTL = 0
+			result.Answers[i].TTL = 0
 		}
 	}
 
@@ -618,9 +618,9 @@ func (w *worker) process(workerTask *workerTask) (processResponse, error) {
 				// TODO responses without at TTL will not be evicted from the cache, so not caching it for now
 				// TODO decide what to do with large responses
 				if len(candidateResponse.Answers) > 0 && len(candidateResponse.Answers) < 1000 {
-					for _, answer := range candidateResponse.Answers {
-						if answer.TTL > w.config.MaxTTL {
-							answer.TTL = w.config.MaxTTL
+					for i := range candidateResponse.Answers {
+						if candidateResponse.Answers[i].TTL > w.config.MaxTTL {
+							candidateResponse.Answers[i].TTL = w.config.MaxTTL
 						}
 					}

Fix Completeness Assessment

The patch completely resolves the value-copy bug for the Answers slice in both rewriteTTLs() and worker.process(). However, developers and auditors reviewing worker.process() should note the explicit architectural constraint visible in the surrounding conditional: if len(candidateResponse.Answers) > 0 && len(candidateResponse.Answers) < 1000. If an upstream DNS response contains 1000 or more answer records, MaxTTL clamping is skipped by design. Furthermore, if DNS authority (Ns) or additional (Extra) resource record sections are cached and served in future iterations of the software, similar index-based TTL adjustments must be applied to those slices as well.

No weaponized exploits or active exploitation campaigns exist for GHSA-9ppr-wfw9-xvr2, as the vulnerability is a functional logic flaw affecting cache integrity rather than a memory corruption or command injection vector. However, the condition is trivially triggered during normal DNS proxy operations and can be abused in scenarios involving infrastructure transitions or malicious upstream servers.

Attack Scenario 1: Bypassing MaxTTL Policy via Malicious Authoritative Server

An administrator configures netfoil with a strict MaxTTL (e.g., 300 seconds) to ensure that DNS mappings cannot remain cached for extended durations. An attacker operating an authoritative DNS server returns an A record with a TTL of 604800 (7 days).

1. A client behind netfoil queries the attacker-controlled domain.
2. netfoil resolves the query and passes the response to worker.process().
3. The MaxTTL loop fails to clamp the 604800 value due to the value-copy bug.
4. netfoil caches the record for the full 7-day duration instead of evicting it after 300 seconds. If the domain is later sinkholed or updated at the parent zone or upstream filtering resolver, netfoil continues serving the attacker's IP for up to 7 days.

Attack Scenario 2: Cascading Stale Cache Retention (Double-Caching Effect)

Even when MaxTTL is not exceeded, the failure to decrement TTLs in rewriteTTLs() extends the effective lifetime of DNS records up to 2x their authoritative TTL across downstream caching layers.

1. At $T = 0$, a client queries example.com (authoritative TTL = 300 seconds). netfoil caches the response.
2. At $T = 290$ seconds, a second downstream stub resolver or OS DNS cache queries netfoil for example.com.
3. Because diffSeconds (290) is less than the original TTL (300), rewriteTTLs() considers the entry valid (ok = true), fails to subtract 290 from the returned TTL, and serves the answer with TTL = 300.
4. The downstream client caches the record for an additional 300 seconds, retaining the mapping until $T = 590$ seconds. If the IP address of example.com was decommissioned at $T = 10$ seconds (e.g., cloud IP release), clients remain vulnerable to IP reuse or subdomain takeover for nearly twice the intended TTL window.

Behavioral Verification Procedure

Security engineers can verify whether a running netfoil instance is affected by issuing two sequential DNS queries separated by a short delay using dig:

# Query 1: Populate netfoil cache and observe initial TTL (e.g., 300s)
dig @<netfoil-ip> -p <netfoil-port> example.com A +noall +answer
 
# Wait 15 seconds
sleep 15
 
# Query 2: Re-query netfoil and inspect the returned TTL
dig @<netfoil-ip> -p <netfoil-port> example.com A +noall +answer

On a vulnerable version (>= 0.3.0, < 0.6.1), Query 2 returns the exact same TTL as Query 1 (300). On a patched version (>= 0.6.1), Query 2 returns a decremented TTL (285).

The security and operational impact of GHSA-9ppr-wfw9-xvr2 centers on DNS cache integrity and policy enforcement. While GitHub did not assign a CVSS score in the advisory metadata, evaluating the flaw under the CVSS v3.1 specification yields a base score of 5.3 (Medium) with vector CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:N.

• Attack Vector (AV:N) & Attack Complexity (AC:L): The DNS proxy processes queries over the network without special conditions or race windows.
• Privileges Required (PR:N) & User Interaction (UI:N): Standard unauthenticated DNS queries trigger the caching logic automatically.
• Integrity (I:L): The proxy modifies transited protocol metadata improperly by emitting inaccurate TTL values that violate RFC 1035 caching semantics and bypass local MaxTTL bounds.
• Confidentiality (C:N) & Availability (A:N): The flaw does not disclose sensitive memory contents or crash the netfoil process, though serving stale IP records during an infrastructure failover can indirectly degrade service reachability for downstream clients.

> [!NOTE] > Because GHSA-9ppr-wfw9-xvr2 was published directly as a GitHub Security Advisory without an associated CVE identifier, it is not indexed by NVD, FIRST EPSS, or the CISA Known Exploited Vulnerabilities (KEV) catalog.

Primary Remediation: Upgrade to v0.6.1

All users and downstream projects embedding or deploying github.com/tinfoil-factory/netfoil should upgrade to version v0.6.1 or later, which incorporates commit ea42977900c864b7b1c42f26c5870c86008118a0.

To update the dependency in a Go module project:

go get github.com/tinfoil-factory/netfoil@v0.6.1
go mod tidy

If deploying netfoil as a standalone binary, rebuild the binary from the v0.6.1 release tag and restart the netfoil service so that any stale in-memory cache structures are flushed.

Workarounds

If an immediate upgrade to v0.6.1 is operationally blocked, administrators can mitigate the issue by disabling response caching in netfoil configuration (if supported by the deployment topology) or restarting the netfoil daemon after critical upstream DNS migrations to forcibly evict cached entries.

Preventing Similar Defects in Go Codebases

To prevent value-copy mutations during slice iteration in Go, engineering teams should enforce static analysis checks in CI/CD pipelines. Standard Go static analysis tools—such as govet and golangci-lint (specifically analyzers checking for dead stores and range value mutations)—detect assignments to fields of range-loop value variables that are never subsequently read.