fix(handler/tun): guard every packet classifier against an empty buffer

waterutil.IsIPv4 and IsIPv6 index packet[0] with no length check, and a
tun device can legally return zero bytes with no error — an Android
VpnService fd does exactly that. routing such a read into a classifier
panics with "index out of range [0] with length 0", and in the c-shared
library the phone loads the runtime cannot unwind past the cgo boundary,
so the panic took the app: the spoke died on 2026-10-12 with three such
lines in crash.log, no stack, and a 17-minute gap before Android
restarted it.

The knowledge was already in the tree — sourceOf and destinationOf in
handler/tun/p2p.go carry the guard, with a comment naming this exact
panic — but the eight reads in the client and server loops and the
association classifier in handler/router did not. The asymmetry was the
bug.

The guard now lives once, in internal/util/tun, as IPv4Packet and
IPv6Packet, and every classifier goes through it. Tested at the real
entry point: a fake device that returns (0, nil) before handing over a
real IPv4 packet, asserted to be survived and forwarded.
This commit is contained in:
ginuerzh
2026-10-13 14:12:50 +00:00
parent 8d253dcd88
commit 993419d845
6 changed files with 161 additions and 10 deletions
+3 -2
View File
@@ -16,6 +16,7 @@ import (
"github.com/go-gost/core/router"
"github.com/go-gost/core/sd"
"github.com/go-gost/relay"
tun_util "github.com/go-gost/x/internal/util/tun"
xip "github.com/go-gost/x/internal/net/ip"
"github.com/go-gost/x/internal/util/cache"
traffic_wrapper "github.com/go-gost/x/limiter/traffic/wrapper"
@@ -234,7 +235,7 @@ func (h *routerHandler) sdRenew(ctx context.Context, clientID string, connectorI
func (h *routerHandler) handlePacket(ctx context.Context, data []byte, routerID relay.TunnelID, log logger.Logger) error {
// ---- Parse IP header ----
var dstIP net.IP
if waterutil.IsIPv4(data) {
if tun_util.IPv4Packet(data) {
header, err := ipv4.ParseHeader(data)
if err != nil {
return err
@@ -247,7 +248,7 @@ func (h *routerHandler) handlePacket(ctx context.Context, data []byte, routerID
header.Src, header.Dst, xip.Protocol(waterutil.IPv4Protocol(data)),
header.Len, header.TotalLen, header.ID, header.Flags)
}
} else if waterutil.IsIPv6(data) {
} else if tun_util.IPv6Packet(data) {
header, err := ipv6.ParseHeader(data)
if err != nil {
return err
+4 -4
View File
@@ -147,7 +147,7 @@ func (h *tunHandler) transportClient(ctx context.Context, tun io.ReadWriter, con
return fmt.Errorf("%w: read: %w", ErrTun, err)
}
if waterutil.IsIPv4(b[:n]) {
if tun_util.IPv4Packet(b[:n]) {
header, err := ipv4.ParseHeader(b[:n])
if err != nil {
log.Warn(err)
@@ -159,7 +159,7 @@ func (h *tunHandler) transportClient(ctx context.Context, tun io.ReadWriter, con
header.Src, header.Dst, xip.Protocol(waterutil.IPv4Protocol(b[:n])),
header.Len, header.TotalLen, header.ID, header.Flags)
}
} else if waterutil.IsIPv6(b[:n]) {
} else if tun_util.IPv6Packet(b[:n]) {
header, err := ipv6.ParseHeader(b[:n])
if err != nil {
log.Warn(err)
@@ -248,7 +248,7 @@ func (h *tunHandler) transportClient(ctx context.Context, tun io.ReadWriter, con
return nil
}
if waterutil.IsIPv4(b[:n]) {
if tun_util.IPv4Packet(b[:n]) {
header, err := ipv4.ParseHeader(b[:n])
if err != nil {
log.Warn(err)
@@ -260,7 +260,7 @@ func (h *tunHandler) transportClient(ctx context.Context, tun io.ReadWriter, con
header.Src, header.Dst, xip.Protocol(waterutil.IPv4Protocol(b[:n])),
header.Len, header.TotalLen, header.ID, header.Flags)
}
} else if waterutil.IsIPv6(b[:n]) {
} else if tun_util.IPv6Packet(b[:n]) {
header, err := ipv6.ParseHeader(b[:n])
if err != nil {
log.Warn(err)
+93
View File
@@ -0,0 +1,93 @@
package tun
import (
"context"
"net"
"testing"
"time"
xlogger "github.com/go-gost/x/logger"
)
// zeroLengthThenPacket is a tun device that hands the reader one empty packet
// before a real one. An Android VpnService fd does exactly that, and the read
// is legal: zero bytes, no error.
//
// This is the shape of the crash that took the spoke down on 2026-10-12. The
// last line the phone logged was a dial inside this relay path
// ("connect /ip", forward/connector.go:44) and crash.log held only
// "panic: runtime error: index out of range [0] with length 0" — because
// waterutil.IsIPv4/IsIPv6 index packet[0] with no length check, so classifying
// an empty read panicked right there. sourceOf/destinationOf (p2p.go) already
// guard the empty buffer; these read loops never did.
type zeroLengthThenPacket struct {
empties int
packet []byte
sent bool
}
func (d *zeroLengthThenPacket) Read(p []byte) (int, error) {
if d.empties > 0 {
d.empties--
return 0, nil
}
if !d.sent && len(d.packet) > 0 {
d.sent = true
return copy(p, d.packet), nil
}
return 0, net.ErrClosed
}
func (d *zeroLengthThenPacket) Write(p []byte) (int, error) { return len(p), nil }
// TestTransportClientSurvivesAnEmptyPacketRead: the relay loop must drop a
// zero-length read and carry on — the packet that follows it still reaches the
// link — instead of panicking inside waterutil on packet[0].
func TestTransportClientSurvivesAnEmptyPacketRead(t *testing.T) {
h := newTestHandler(nil, nil)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
dev := &zeroLengthThenPacket{empties: 1, packet: realIPv4Packet()}
ccA, ccB := net.Pipe()
defer ccA.Close()
defer ccB.Close()
// Whatever the relay writes to the link, collected here: the proof that the
// loop was still running after the empty read.
got := make(chan []byte, 1)
go func() {
buf := make([]byte, 256)
ccB.SetReadDeadline(time.Now().Add(3 * time.Second))
n, err := ccB.Read(buf)
if err != nil && n <= 0 {
got <- nil
return
}
got <- append([]byte{}, buf[:n]...)
}()
panicked := make(chan any, 1)
errCh := make(chan error, 1)
go func() {
defer func() {
if r := recover(); r != nil {
panicked <- r
}
}()
errCh <- h.transportClient(ctx, dev, ccA, xlogger.Nop())
}()
select {
case r := <-panicked:
t.Fatalf("transportClient panicked on an empty packet read: %v", r)
case pkt := <-got:
if len(pkt) == 0 {
t.Fatal("nothing reached the link: the loop died with the empty read")
}
case err := <-errCh:
t.Fatalf("transportClient returned (%v) instead of forwarding the packet after the empty read", err)
case <-time.After(5 * time.Second):
t.Fatal("timed out waiting for the relay to forward the packet")
}
}
+4 -4
View File
@@ -64,7 +64,7 @@ func (h *tunHandler) transportServer(ctx context.Context, tun io.ReadWriter, con
}
var src, dst net.IP
if waterutil.IsIPv4(b[:n]) {
if tun_util.IPv4Packet(b[:n]) {
header, err := ipv4.ParseHeader(b[:n])
if err != nil {
log.Warnf("parse ipv4 packet header: %v", err)
@@ -77,7 +77,7 @@ func (h *tunHandler) transportServer(ctx context.Context, tun io.ReadWriter, con
src, dst, xip.Protocol(waterutil.IPv4Protocol(b[:n])),
header.Len, header.TotalLen, header.ID, header.Flags)
}
} else if waterutil.IsIPv6(b[:n]) {
} else if tun_util.IPv6Packet(b[:n]) {
header, err := ipv6.ParseHeader(b[:n])
if err != nil {
log.Warnf("parse ipv6 packet header: %v", err)
@@ -189,7 +189,7 @@ func (h *tunHandler) transportServer(ctx context.Context, tun io.ReadWriter, con
}
var src, dst net.IP
if waterutil.IsIPv4(b[:n]) {
if tun_util.IPv4Packet(b[:n]) {
header, err := ipv4.ParseHeader(b[:n])
if err != nil {
log.Warnf("parse ipv4 packet header: %v", err)
@@ -202,7 +202,7 @@ func (h *tunHandler) transportServer(ctx context.Context, tun io.ReadWriter, con
src, dst, xip.Protocol(waterutil.IPv4Protocol(b[:n])),
header.Len, header.TotalLen, header.ID, header.Flags)
}
} else if waterutil.IsIPv6(b[:n]) {
} else if tun_util.IPv6Packet(b[:n]) {
header, err := ipv6.ParseHeader(b[:n])
if err != nil {
log.Warnf("parse ipv6 packet header: %v", err)
+21
View File
@@ -0,0 +1,21 @@
package tun
import "github.com/songgao/water/waterutil"
// IPv4Packet and IPv6Packet are the only ways a reader should ask waterutil
// which family a buffer is.
//
// waterutil.IsIPv4/IsIPv6 index packet[0] with no length check, so an empty
// buffer panics there instead of reporting "neither". An empty buffer is not
// hypothetical: a device may legally return zero bytes with no error — an
// Android VpnService tun fd does exactly that — and that read produced the
// "panic: runtime error: index out of range [0] with length 0" that took the
// spoke down on 2026-10-12, three times, with no stack left in crash.log. In a
// c-shared library the runtime cannot unwind past the cgo boundary, so the
// panic took the app with it.
//
// Every packet classifier needs this guard: the read loops in handler/tun,
// the association classifier in handler/router, and anything added later.
func IPv4Packet(b []byte) bool { return len(b) > 0 && waterutil.IsIPv4(b) }
func IPv6Packet(b []byte) bool { return len(b) > 0 && waterutil.IsIPv6(b) }
+36
View File
@@ -0,0 +1,36 @@
package tun
import "testing"
// TestPacketFamilyGuardsTheEmptyBuffer pins the guard every packet classifier
// in this repo must carry. waterutil.IsIPv4/IsIPv6 index packet[0] with no
// length check, so an empty buffer — a device's legal zero-byte read — panics
// with "index out of range [0] with length 0": the panic that took the spoke
// down on 2026-10-12, in a c-shared library where nothing could catch it.
func TestPacketFamilyGuardsTheEmptyBuffer(t *testing.T) {
if IPv4Packet(nil) || IPv4Packet([]byte{}) {
t.Error("an empty buffer is not an IPv4 packet")
}
if IPv6Packet(nil) || IPv6Packet([]byte{}) {
t.Error("an empty buffer is not an IPv6 packet")
}
// The guard must not reject the packets it reports on: a zero-length buffer
// is the only case it answers "no" for.
v4 := []byte{0x45, 0, 0, 20, 0, 0, 0, 0, 0, 1}
if !IPv4Packet(v4) {
t.Error("a real IPv4 header is not reported as one")
}
if IPv6Packet(v4) {
t.Error("an IPv4 header is reported as IPv6")
}
v6 := make([]byte, 40)
v6[0] = 0x60
if !IPv6Packet(v6) {
t.Error("a real IPv6 header is not reported as one")
}
if IPv4Packet(v6) {
t.Error("an IPv6 header is reported as IPv4")
}
}