fix(wrapper): prevent nil-pointer dereference in WrapUDPConn chain when stats is nil
stats.WrapUDPConn returned nil when pStats was nil (introduced in 7c95d40),
but callers in the kcp and rudp listeners passed the result directly to
admission.WrapUDPConn and limiter.WrapUDPConn, which wrapped a nil PacketConn.
When service.Serve() called conn.LocalAddr(), the auto-generated method
delegated to the nil PacketConn, causing a panic.
Root cause: WrapUDPConn behavior was inconsistent with WrapConn and
WrapPacketConn in the same package, which both return the original connection
instead of nil when the optional config is absent.
Fixes:
1. listener/kcp, listener/rudp — guard stats.WrapUDPConn with Stats != nil
(consistent with quic and wt listeners which already had this guard).
2. observer/stats/wrapper — when pStats is nil, pass through the original
connection instead of returning nil (matches WrapConn/WrapPacketConn).
3. admission/wrapper, limiter/traffic/wrapper — add defensive nil guards:
nil pc → return nil; nil config → pass through original connection.
All four WrapUDPConn implementations now share the same nil semantics.
4. listener/udp_wrapper_safety_test.go — regression test that verifies the
full metrics→stats→admission→limiter chain with all optional configs nil
does not produce a wrapper with a nil internal PacketConn.
Fixes: go-gost/gost#873
This commit is contained in:
@@ -163,10 +163,20 @@ type udpConn struct {
|
||||
// WrapUDPConn wraps a net.PacketConn as a udp.Conn with admission control.
|
||||
// This is used in UDP forwarding paths where the connection is treated
|
||||
// as a connected UDP socket.
|
||||
func WrapUDPConn(admission admission.Admission, pc net.PacketConn) udp.Conn {
|
||||
// If pc is nil, nil is returned. If admission is nil, the original connection
|
||||
// is returned unchanged (no-op).
|
||||
func WrapUDPConn(adm admission.Admission, pc net.PacketConn) udp.Conn {
|
||||
if pc == nil {
|
||||
return nil
|
||||
}
|
||||
if adm == nil {
|
||||
if uc, ok := pc.(udp.Conn); ok {
|
||||
return uc
|
||||
}
|
||||
}
|
||||
return &udpConn{
|
||||
PacketConn: pc,
|
||||
admission: admission,
|
||||
admission: adm,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -227,10 +227,20 @@ func (c *udpConn) DroppedPackets() int64 {
|
||||
}
|
||||
|
||||
// WrapUDPConn wraps a net.PacketConn as a udp.Conn with traffic rate limiting.
|
||||
func WrapUDPConn(pc net.PacketConn, limiter traffic.TrafficLimiter, key string, opts ...limiter.Option) udp.Conn {
|
||||
// If pc is nil, nil is returned. If limiter is nil, the original connection is
|
||||
// returned unchanged (no-op).
|
||||
func WrapUDPConn(pc net.PacketConn, lim traffic.TrafficLimiter, key string, opts ...limiter.Option) udp.Conn {
|
||||
if pc == nil {
|
||||
return nil
|
||||
}
|
||||
if lim == nil {
|
||||
if uc, ok := pc.(udp.Conn); ok {
|
||||
return uc
|
||||
}
|
||||
}
|
||||
return &udpConn{
|
||||
PacketConn: pc,
|
||||
limiter: limiter,
|
||||
limiter: lim,
|
||||
opts: opts,
|
||||
key: key,
|
||||
}
|
||||
|
||||
@@ -78,7 +78,9 @@ func (l *kcpListener) Init(md md.Metadata) (err error) {
|
||||
}
|
||||
|
||||
conn = metrics.WrapUDPConn(l.options.Service, conn)
|
||||
if l.options.Stats != nil {
|
||||
conn = stats.WrapUDPConn(conn, l.options.Stats)
|
||||
}
|
||||
conn = admission.WrapUDPConn(l.options.Admission, conn)
|
||||
conn = limiter_wrapper.WrapUDPConn(
|
||||
conn,
|
||||
|
||||
@@ -106,7 +106,9 @@ func (l *rudpListener) Accept() (conn net.Conn, err error) {
|
||||
|
||||
if pc, ok := conn.(net.PacketConn); ok {
|
||||
uc := metrics.WrapUDPConn(l.options.Service, pc)
|
||||
if l.options.Stats != nil {
|
||||
uc = stats.WrapUDPConn(uc, l.options.Stats)
|
||||
}
|
||||
uc = admission.WrapUDPConn(l.options.Admission, uc)
|
||||
conn = limiter_wrapper.WrapUDPConn(
|
||||
uc,
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
package listener_test
|
||||
|
||||
import (
|
||||
"net"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
admission_wrapper "github.com/go-gost/x/admission/wrapper"
|
||||
limiter_wrapper "github.com/go-gost/x/limiter/traffic/wrapper"
|
||||
metrics_wrapper "github.com/go-gost/x/metrics/wrapper"
|
||||
stats_wrapper "github.com/go-gost/x/observer/stats/wrapper"
|
||||
)
|
||||
|
||||
// TestWrapUDPConnChainNoCrashWithNilConfigs verifies that chaining
|
||||
// WrapUDPConn calls with nil optional configs (stats, admission, limiter)
|
||||
// does not produce a wrapper with a nil internal PacketConn.
|
||||
//
|
||||
// Regression test for go-gost/gost#873 — stats.WrapUDPConn returning nil
|
||||
// when stats is nil, which then entered admission.WrapUDPConn with a nil
|
||||
// PacketConn, causing a nil-pointer dereference in service.Serve().
|
||||
func TestWrapUDPConnChainNoCrashWithNilConfigs(t *testing.T) {
|
||||
// Simulate a listener's Init() chaining: metrics → stats → admission → limiter.
|
||||
pc := &fakePacketConn{}
|
||||
|
||||
// Step 1: metrics.WrapUDPConn — always returns non-nil for non-nil input.
|
||||
uc := metrics_wrapper.WrapUDPConn("test-service", pc)
|
||||
if uc == nil {
|
||||
t.Fatal("metrics.WrapUDPConn returned nil for non-nil input")
|
||||
}
|
||||
if uc.LocalAddr() == nil {
|
||||
t.Error("LocalAddr() returned nil after metrics wrapper")
|
||||
}
|
||||
|
||||
// Step 2: stats.WrapUDPConn with nil stats — must NOT return nil.
|
||||
uc = stats_wrapper.WrapUDPConn(uc, nil)
|
||||
if uc == nil {
|
||||
t.Fatal("stats.WrapUDPConn returned nil for nil stats (bug — should return original)")
|
||||
}
|
||||
if uc.LocalAddr() == nil {
|
||||
t.Error("LocalAddr() returned nil after stats wrapper with nil stats")
|
||||
}
|
||||
|
||||
// Step 3: admission.WrapUDPConn with nil admission — must NOT return nil.
|
||||
uc = admission_wrapper.WrapUDPConn(nil, uc)
|
||||
if uc == nil {
|
||||
t.Fatal("admission.WrapUDPConn returned nil for nil admission (bug — should return original)")
|
||||
}
|
||||
if uc.LocalAddr() == nil {
|
||||
t.Error("LocalAddr() returned nil after admission wrapper with nil admission")
|
||||
}
|
||||
|
||||
// Step 4: limiter.WrapUDPConn with nil limiter — must NOT return nil.
|
||||
uc = limiter_wrapper.WrapUDPConn(uc, nil, "key")
|
||||
if uc == nil {
|
||||
t.Fatal("limiter.WrapUDPConn returned nil for nil limiter (bug — should return original)")
|
||||
}
|
||||
if uc.LocalAddr() == nil {
|
||||
t.Error("LocalAddr() returned nil after limiter wrapper with nil limiter")
|
||||
}
|
||||
}
|
||||
|
||||
// TestWrapUDPConnNilInputReturnsNil verifies that all WrapUDPConn variants
|
||||
// return nil when given a nil net.PacketConn (defensive guard).
|
||||
func TestWrapUDPConnNilInputReturnsNil(t *testing.T) {
|
||||
if v := metrics_wrapper.WrapUDPConn("test", nil); v != nil {
|
||||
t.Error("metrics.WrapUDPConn(nil) should return nil")
|
||||
}
|
||||
if v := stats_wrapper.WrapUDPConn(nil, nil); v != nil {
|
||||
t.Error("stats.WrapUDPConn(nil, nil) should return nil")
|
||||
}
|
||||
if v := admission_wrapper.WrapUDPConn(nil, nil); v != nil {
|
||||
t.Error("admission.WrapUDPConn(nil, nil) should return nil")
|
||||
}
|
||||
if v := limiter_wrapper.WrapUDPConn(nil, nil, ""); v != nil {
|
||||
t.Error("limiter.WrapUDPConn(nil, nil, '') should return nil")
|
||||
}
|
||||
}
|
||||
|
||||
// fakePacketConn is a minimal net.PacketConn for testing wrapper chains.
|
||||
type fakePacketConn struct{}
|
||||
|
||||
func (c *fakePacketConn) ReadFrom(p []byte) (n int, addr net.Addr, err error) {
|
||||
return 0, nil, nil
|
||||
}
|
||||
func (c *fakePacketConn) WriteTo(p []byte, addr net.Addr) (n int, err error) {
|
||||
return 0, nil
|
||||
}
|
||||
func (c *fakePacketConn) Close() error { return nil }
|
||||
func (c *fakePacketConn) LocalAddr() net.Addr { return &fakeAddr{} }
|
||||
func (c *fakePacketConn) SetDeadline(t time.Time) error { return nil }
|
||||
func (c *fakePacketConn) SetReadDeadline(t time.Time) error { return nil }
|
||||
func (c *fakePacketConn) SetWriteDeadline(t time.Time) error { return nil }
|
||||
|
||||
type fakeAddr struct{}
|
||||
|
||||
func (a *fakeAddr) Network() string { return "udp" }
|
||||
func (a *fakeAddr) String() string { return "127.0.0.1:0" }
|
||||
@@ -156,10 +156,18 @@ type udpConn struct {
|
||||
// accepted UDP client session == one connection, mirroring WrapConn) and
|
||||
// tracks bytes read and written across all read/write methods (Read, Write,
|
||||
// ReadFrom, WriteTo, ReadFromUDP, ReadMsgUDP, WriteToUDP, WriteMsgUDP). On
|
||||
// Close, it decrements the current connection count. If pc or pStats is nil,
|
||||
// it returns nil.
|
||||
// Close, it decrements the current connection count.
|
||||
//
|
||||
// If pc is nil, nil is returned. If pStats is nil, the original connection
|
||||
// is returned unchanged (consistent with WrapConn and WrapPacketConn).
|
||||
func WrapUDPConn(pc net.PacketConn, pStats stats.Stats) udp.Conn {
|
||||
if pc == nil || pStats == nil {
|
||||
if pc == nil {
|
||||
return nil
|
||||
}
|
||||
if pStats == nil {
|
||||
if uc, ok := pc.(udp.Conn); ok {
|
||||
return uc
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user