Skip to content

Commit af82b22

Browse files
committed
fix: guard RemoteAddr parsing against out-of-bounds slice panic
addKnownVariablesToServer stripped IPv6 brackets with an unchecked slice (ip[1:len(ip)-1]). A malformed RemoteAddr such as "[" or "[:9000" produced a 1-byte ip and panicked. This runs inside the go_register_server_variables cgo callback, where a Go panic cannot unwind through the C frame and aborts the whole process rather than failing a single request. Extract splitRemoteAddr, which parses via net.SplitHostPort and falls back to a bounds-safe lenient split that only strips brackets when both are present. Behavior for well-formed addresses is unchanged.
1 parent 8c9e331 commit af82b22

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)