Skip to content

Commit a9893b4

Browse files
authored
fix: guard RemoteAddr parsing against out-of-bounds slice panic (#2463)
`addKnownVariablesToServer` stripped IPv6 brackets with an unchecked slice, so a malformed `RemoteAddr` like `"["` or `"[:9000"` panicked. This runs in the `go_register_server_variables` cgo callback, where a panic can't cross the C frame and crashes the whole process. Not reachable behind stock Caddy; reachable for library callers and real-IP rewrites. Fix: parse via `net.SplitHostPort` with a bounds-safe fallback. `TestSplitRemoteAddr` covers the crashing inputs; well-formed addresses are unchanged.
1 parent ccb268d commit a9893b4

2 files changed

Lines changed: 54 additions & 12 deletions

File tree

cgi.go

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,7 @@ var cStringHTTPMethods = map[string]*C.char{
4545
func addKnownVariablesToServer(fc *frankenPHPContext, trackVarsArray *C.zval) {
4646
request := fc.request
4747
// Separate remote IP and port; more lenient than net.SplitHostPort
48-
var ip, port string
49-
if idx := strings.LastIndex(request.RemoteAddr, ":"); idx > -1 {
50-
ip = request.RemoteAddr[:idx]
51-
port = request.RemoteAddr[idx+1:]
52-
} else {
53-
ip = request.RemoteAddr
54-
}
55-
56-
// Remove [] from IPv6 addresses
57-
if len(ip) > 0 && ip[0] == '[' {
58-
ip = ip[1 : len(ip)-1]
59-
}
48+
ip, port := splitRemoteAddr(request.RemoteAddr)
6049

6150
var rs, https, sslProtocol *C.zend_string
6251
var sslCipher string
@@ -354,6 +343,28 @@ func sanitizedPathJoin(root, reqPath string) string {
354343
return path
355344
}
356345

346+
// splitRemoteAddr splits "host:port" leniently: a missing port is accepted.
347+
// A malformed value such as "[" must not panic, as that would unwind out of
348+
// the go_register_server_variables cgo callback and crash the whole process.
349+
func splitRemoteAddr(remoteAddr string) (ip, port string) {
350+
if host, p, err := net.SplitHostPort(remoteAddr); err == nil {
351+
return host, p
352+
}
353+
354+
if idx := strings.LastIndex(remoteAddr, ":"); idx > -1 {
355+
ip = remoteAddr[:idx]
356+
port = remoteAddr[idx+1:]
357+
} else {
358+
ip = remoteAddr
359+
}
360+
361+
if len(ip) >= 2 && ip[0] == '[' && ip[len(ip)-1] == ']' {
362+
ip = ip[1 : len(ip)-1]
363+
}
364+
365+
return ip, port
366+
}
367+
357368
const separator = string(filepath.Separator)
358369

359370
func ensureLeadingSlash(path string) string {

cgi_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,37 @@ func TestEnsureLeadingSlash(t *testing.T) {
3333
}
3434
}
3535

36+
func TestSplitRemoteAddr(t *testing.T) {
37+
t.Parallel()
38+
39+
tests := []struct {
40+
name string
41+
addr string
42+
wantIP string
43+
wantPort string
44+
}{
45+
{"ipv4 with port", "1.2.3.4:5", "1.2.3.4", "5"},
46+
{"ipv6 bracketed with port", "[::1]:443", "::1", "443"},
47+
{"ipv6 zone bracketed with port", "[fe80::1%eth0]:443", "fe80::1%eth0", "443"},
48+
{"ipv4 without port", "192.168.0.1", "192.168.0.1", ""},
49+
{"empty", "", "", ""},
50+
// Must not panic: would crash the process via the cgo callback.
51+
{"lone open bracket", "[", "[", ""},
52+
{"open bracket with port", "[:9000", "[", "9000"},
53+
{"empty brackets", "[]", "", ""},
54+
}
55+
56+
for _, tt := range tests {
57+
t.Run(tt.name, func(t *testing.T) {
58+
t.Parallel()
59+
60+
ip, port := splitRemoteAddr(tt.addr)
61+
assert.Equal(t, tt.wantIP, ip, "splitRemoteAddr(%q) ip", tt.addr)
62+
assert.Equal(t, tt.wantPort, port, "splitRemoteAddr(%q) port", tt.addr)
63+
})
64+
}
65+
}
66+
3667
func TestSplitPos(t *testing.T) {
3768
tests := []struct {
3869
name string

0 commit comments

Comments
 (0)