dynamic upstream in Caddyfile without versions leads to nil pointer dereference

[FossoresLP]

When running a reverse proxy with a dynamic upstream without explicitly specifying the IP versions to use does not (as documented) fall back to using both IPv4 and IPv6 but instead leads to a nil pointer dereference.

Caddyfile with issue:

localhost {
	reverse_proxy {
		dynamic a {
			name example.com
			port 80
		}
	}
}

Caddyfile without issue:

localhost {
	reverse_proxy {
		dynamic a {
			name example.com
			port 80
			versions ipv4 ipv6
		}
	}
}

The issue is caused by a nil pointer dereference in line 300 of modules/caddyhttp/reverseproxy/upstreams.go because AUpstreams.Versions is nil.

A quick fix would be to add a nil check before accessing AUpstreams.Versions:

diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go
index 09efdd51..7cbcae7e 100644
--- a/modules/caddyhttp/reverseproxy/upstreams.go
+++ b/modules/caddyhttp/reverseproxy/upstreams.go
@@ -297,8 +297,15 @@ func (au *AUpstreams) Provision(_ caddy.Context) error {
 func (au AUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) {
 	repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
 
-	resolveIpv4 := au.Versions.IPv4 == nil || *au.Versions.IPv4
-	resolveIpv6 := au.Versions.IPv6 == nil || *au.Versions.IPv6
+	var resolveIpv4, resolveIpv6 bool
+
+	if au.Versions == nil {
+		resolveIpv4 = true
+		resolveIpv6 = true
+	} else {
+		resolveIpv4 = au.Versions.IPv4 == nil || *au.Versions.IPv4
+		resolveIpv6 = au.Versions.IPv6 == nil || *au.Versions.IPv6
+	}
 
 	// Map ipVersion early, so we can use it as part of the cache-key.
 	// This should be fairly inexpensive and comes and the upside of

Tested versions:

• 2.7.4 from caddyserver.com
• 2.7.4 from source
• current master 2cac3c5491e6428441ecf668cc4f5a86e67ed9b3
  Go version: 1.21.1

Backtrace

2023/09/09 18:51:46.084	DEBUG	http.stdlib	http2: panic serving [::1]:49536: runtime error: invalid memory address or nil pointer dereference
goroutine 65 [running]:
golang.org/x/net/http2.(*serverConn).runHandler.func1()
	golang.org/x/[email protected]/http2/server.go:2307 +0x13b
panic({0x199a120?, 0x36807c0?})
	runtime/panic.go:914 +0x21f
github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy.AUpstreams.GetUpstreams({{0xc00035e620, 0xb}, {0xc00035e62c, 0x2}, 0xdf8475800, 0x0, 0x0, 0x0, 0x0, 0x36c0ce0}, ...)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/reverseproxy/upstreams.go:300 +0xb1
github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy.(*Handler).proxyLoopIteration(0xc000d4e780, 0xc000352900, 0x0?, {0x292aa90, 0xc000704118}, {0x0, 0x0}, {0xc1374d9085054a95, 0x1712a28bc, 0x36c25a0}, ...)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/reverseproxy/reverseproxy.go:460 +0xd5
github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy.(*Handler).ServeHTTP(0xc000d4e780, {0x292aa90, 0xc000704118}, 0xc000352800, {0x2918960, 0x2694c00})
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/reverseproxy/reverseproxy.go:436 +0x3f3
github.com/caddyserver/caddy/v2/modules/caddyhttp.wrapMiddleware.func1.1({0x292aa90?, 0xc000704118?}, 0x2918960?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/routes.go:331 +0x3a
github.com/caddyserver/caddy/v2/modules/caddyhttp.HandlerFunc.ServeHTTP(0x2918960?, {0x292aa90?, 0xc000704118?}, 0x0?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/caddyhttp.go:58 +0x29
github.com/caddyserver/caddy/v2/modules/caddyhttp.RouteList.Compile.wrapRoute.func1.1({0x292aa90, 0xc000704118}, 0x1?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/routes.go:300 +0x2c9
github.com/caddyserver/caddy/v2/modules/caddyhttp.HandlerFunc.ServeHTTP(0xc000ed0120?, {0x292aa90?, 0xc000704118?}, 0x2918960?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/caddyhttp.go:58 +0x29
github.com/caddyserver/caddy/v2/modules/caddyhttp.(*Subroute).ServeHTTP(0xc0004814e0, {0x292aa90, 0xc000704118}, 0x18e5801?, {0x2918960, 0x2694c00})
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/subroute.go:74 +0x67
github.com/caddyserver/caddy/v2/modules/caddyhttp.wrapMiddleware.func1.1({0x292aa90?, 0xc000704118?}, 0x2918960?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/routes.go:331 +0x3a
github.com/caddyserver/caddy/v2/modules/caddyhttp.HandlerFunc.ServeHTTP(0x2918960?, {0x292aa90?, 0xc000704118?}, 0xc000320380?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/caddyhttp.go:58 +0x29
github.com/caddyserver/caddy/v2/modules/caddyhttp.RouteList.Compile.wrapRoute.func1.1({0x292aa90, 0xc000704118}, 0x30?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/routes.go:300 +0x2c9
github.com/caddyserver/caddy/v2/modules/caddyhttp.HandlerFunc.ServeHTTP(0xc001302700?, {0x292aa90?, 0xc000704118?}, 0x1b7bba0?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/caddyhttp.go:58 +0x29
github.com/caddyserver/caddy/v2/modules/caddyhttp.(*Server).enforcementHandler(0x7f3a1c08a788?, {0x292aa90?, 0xc000704118?}, 0xc00008c800?, {0x2918960?, 0xc0004815e0?})
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/server.go:430 +0x248
github.com/caddyserver/caddy/v2/modules/caddyhttp.(*App).Provision.(*Server).wrapPrimaryRoute.func1({0x292aa90?, 0xc000704118?}, 0x4d1c2f?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/server.go:406 +0x35
github.com/caddyserver/caddy/v2/modules/caddyhttp.HandlerFunc.ServeHTTP(0xc0002330a0?, {0x292aa90?, 0xc000704118?}, 0xc000352800?)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/caddyhttp.go:58 +0x29
github.com/caddyserver/caddy/v2/modules/caddyhttp.(*Server).ServeHTTP(0xc000d32840, {0x292aa90, 0xc000704118}, 0xc000352600)
	github.com/caddyserver/caddy/[email protected]/modules/caddyhttp/server.go:342 +0xf2e
net/http.serverHandler.ServeHTTP({0xc00013f500?}, {0x292aa90?, 0xc000704118?}, 0x0?)
	net/http/server.go:2938 +0x8e
net/http.initALPNRequest.ServeHTTP({{0x292ea60?, 0xc0012afec0?}, 0xc00013f500?, {0xc000dd8000?}}, {0x292aa90, 0xc000704118}, 0xc000352600)
	net/http/server.go:3546 +0x231
golang.org/x/net/http2.(*serverConn).runHandler(0x36f3460?, 0x0?, 0x0?, 0xc00025e300?)
	golang.org/x/[email protected]/http2/server.go:2314 +0x78
created by golang.org/x/net/http2.(*serverConn).processHeaders in goroutine 59
	golang.org/x/[email protected]/http2/server.go:2028 +0x691

[francislavoie]

Oops! 😅 PR is welcome 👍

I'd probably do it with resolveIpv4 := au.Versions == nil || au.Versions.IPv4 == nil || *au.Versions.IPv4 instead of hoisting vars though.