tcp: Reject SYN on synchronized connections; fix maxSend underflow (#44)

* tcp: add test replicating bugs and 32bit panic with GOARCH=386

* tcp: reject SYN on synchronized connections; fix maxSend underflow
This commit is contained in:
Pat Whittingslow
2026-02-27 16:38:46 +01:00
committed by GitHub
parent f9748da94f
commit eab43c4653
4 changed files with 242 additions and 6 deletions
+1 -1
View File
@@ -1,4 +1,4 @@
//go:build linux && !tinygo
//go:build linux && !tinygo && amd64
package internal
+17 -4
View File
@@ -134,7 +134,12 @@ func (snd *sendSpace) inFlight() Size {
// maxSend returns maximum segment datalength receivable by remote peer.
func (snd *sendSpace) maxSend() Size {
return snd.WND - snd.inFlight()
if inf := snd.inFlight(); inf >= snd.WND {
// Guard uint32 underflow when window shrinks below inflight.
return 0
} else {
return snd.WND - inf
}
}
// recvSpace contains Receive Sequence Space data. Its sequence numbers correspond to remote data.
@@ -252,6 +257,13 @@ func (tcb *ControlBlock) Recv(seg Segment) (err error) {
return err
}
// RFC 9293 §3.10.7.4: SYN on synchronized connection → challenge ACK.
if seg.Flags.HasAny(FlagSYN) && !tcb._state.IsPreestablished() {
tcb.challengeAck = true
tcb.pending[0] |= FlagACK
return errDropSegment
}
prevNxt := tcb.snd.NXT
var pending Flags
switch tcb._state {
@@ -402,10 +414,11 @@ func (tcb *ControlBlock) validateOutgoingSegment(seg Segment) (err error) {
func (tcb *ControlBlock) validateIncomingSegment(seg Segment) (err error) {
flags := seg.Flags
hasAck := flags.HasAll(FlagACK)
// Short circuit SEQ checks if SYN present since the incoming segment initialize1s connection.
checkSEQ := !flags.HasAny(FlagSYN)
established := tcb._state == StateEstablished
// Short circuit SEQ checks if SYN present in pre-established states only.
// In synchronized states SYN must pass normal SEQ validation (RFC 9293 §3.10.7.4).
preestablished := tcb._state.IsPreestablished()
checkSEQ := !flags.HasAny(FlagSYN) || !preestablished
established := tcb._state == StateEstablished
acksOld := hasAck && !tcb.snd.UNA.LessThan(seg.ACK)
acksUnsentData := hasAck && !seg.ACK.LessThanEq(tcb.snd.NXT)
ctlOrDataSegment := established && (seg.DATALEN > 0 || flags.HasAny(FlagFIN|FlagRST))
-1
View File
@@ -62,7 +62,6 @@ func (tcb *ControlBlock) rcvSynRcvd(seg Segment) (pending Flags, err error) {
func (tcb *ControlBlock) rcvEstablished(seg Segment) (pending Flags, err error) {
flags := seg.Flags
dataToAck := seg.DATALEN > 0
hasFin := flags.HasAny(FlagFIN)
if dataToAck || hasFin {
+224
View File
@@ -0,0 +1,224 @@
package tcp
// Tests for the "seqs: bad calculation" panic bug caused by a retransmitted SYN
// being accepted on an ESTABLISHED connection.
//
// Bug 1: validateIncomingSegment skips SEQ checks for any SYN, even in synchronized states.
// Bug 2: rcvEstablished silently accepts bare SYN segments (no data, no FIN).
// Combined: SYN overwrites snd.WND with a small value while data is in flight, causing
// uint32 underflow in maxSend() → panic in PendingSegment.
import "testing"
// TestSYNOnEstablished_PanicRegression replicates the exact crash sequence from the bug report:
// a retransmitted SYN arrives on an ESTABLISHED connection with 1200 bytes in flight,
// clobbers snd.WND to 1025, and triggers "seqs: bad calculation" in PendingSegment.
// To trigger run with GOARCH=386
func TestSYNOnEstablished_PanicRegression(t *testing.T) {
// Values from the bug report timeline.
const (
serverISS Value = 155000
clientISS Value = 4189094524 // The SYN's sequence number from the log.
serverWND Size = 1024
clientWND Size = 1025 // The SYN's small window.
payloadSent Size = 1200 // HTTP response bytes in flight.
)
// Recv must reject the SYN on an ESTABLISHED connection.
t.Run("Recv_rejects_SYN", func(t *testing.T) {
var tcb ControlBlock
tcb.HelperInitState(StateEstablished, serverISS, serverISS+1+Value(payloadSent), serverWND)
tcb.HelperInitRcv(clientISS, clientISS+1, 65535)
tcb.snd.UNA = serverISS + 1
syn := Segment{
SEQ: clientISS,
ACK: 0,
Flags: FlagSYN,
WND: clientWND,
}
err := tcb.Recv(syn)
if err == nil {
t.Fatal("Recv accepted SYN on ESTABLISHED connection; expected error")
}
if tcb.State() != StateEstablished {
t.Fatalf("state changed to %s; want ESTABLISHED", tcb.State())
}
if tcb.snd.WND == clientWND {
t.Fatalf("snd.WND was clobbered to %d by the SYN segment", clientWND)
}
})
// Directly simulate the corrupted state to trigger the panic in PendingSegment.
// This is the actual crash path: inFlight=1200, snd.WND=1025 → maxSend() underflows.
// On 32-bit targets int(underflowedUint32) wraps negative → panic.
t.Run("PendingSegment_panic", func(t *testing.T) {
var tcb ControlBlock
tcb.HelperInitState(StateEstablished, serverISS, serverISS+1+Value(payloadSent), serverWND)
tcb.HelperInitRcv(clientISS, clientISS+1, 65535)
tcb.snd.UNA = serverISS + 1
// Simulate what Recv does when the SYN is accepted (the bug):
// snd.WND gets overwritten with the SYN's small window.
tcb.snd.WND = clientWND // inFlight=1200 > snd.WND=1025 → underflow.
defer func() {
if r := recover(); r != nil {
t.Errorf("confirmed panic: %v", r)
}
}()
tcb.PendingSegment(1) // Any payload > 0 triggers the panic.
})
}
// TestSYNOnEstablished_SEQValidation verifies that SYN segments are subject to
// sequence number validation in synchronized states (Bug 1 from the report).
func TestSYNOnEstablished_SEQValidation(t *testing.T) {
const (
iss Value = 100
remoteISS Value = 5000
window Size = 1000
)
var tcb ControlBlock
tcb.HelperInitState(StateEstablished, iss, iss, window)
tcb.HelperInitRcv(remoteISS, remoteISS, window)
// SYN with SEQ outside the receive window should be rejected.
outOfWindowSYN := Segment{
SEQ: remoteISS - 2000, // Way before rcv.NXT.
Flags: FlagSYN,
WND: 1025,
}
err := tcb.Recv(outOfWindowSYN)
if err == nil {
t.Fatal("SYN with out-of-window SEQ was accepted in ESTABLISHED state")
}
// SYN with SEQ inside the window but not equal to rcv.NXT should also be rejected
// (we require sequential segments per SHLD-31).
inWindowSYN := Segment{
SEQ: remoteISS + 1, // In window but not NXT.
Flags: FlagSYN,
WND: 1025,
}
err = tcb.Recv(inWindowSYN)
if err == nil {
t.Fatal("SYN with in-window non-NXT SEQ was accepted in ESTABLISHED state")
}
}
// TestSYNOnEstablished_ChallengeACK verifies that per RFC 9293 §3.10.7.4,
// receiving a SYN on an ESTABLISHED connection results in a challenge ACK
// rather than silently accepting or resetting (Bug 2 from the report).
func TestSYNOnEstablished_ChallengeACK(t *testing.T) {
const (
issA Value = 100
issB Value = 300
window Size = 1000
)
var tcb ControlBlock
tcb.HelperInitState(StateEstablished, issA, issA, window)
tcb.HelperInitRcv(issB, issB, window)
// SYN at exact rcv.NXT (in-window).
syn := Segment{
SEQ: issB,
Flags: FlagSYN,
WND: 512,
}
err := tcb.Recv(syn)
// Must be rejected (either via validation or rcvEstablished).
if err == nil {
t.Fatal("SYN at rcv.NXT was accepted in ESTABLISHED state; expected rejection")
}
// Connection must stay ESTABLISHED.
if tcb.State() != StateEstablished {
t.Fatalf("state = %s; want ESTABLISHED", tcb.State())
}
// snd.WND must be preserved.
if tcb.snd.WND == 512 {
t.Fatal("snd.WND was overwritten by the SYN's window value")
}
// A challenge ACK should be pending.
seg, ok := tcb.PendingSegment(0)
if !ok {
t.Fatal("no pending segment after SYN on ESTABLISHED; expected challenge ACK")
}
if seg.Flags != FlagACK {
t.Errorf("pending flags = %s; want ACK (challenge ACK)", seg.Flags)
}
if seg.SEQ != issA {
t.Errorf("challenge ACK SEQ = %d; want snd.NXT=%d", seg.SEQ, issA)
}
if seg.ACK != issB {
t.Errorf("challenge ACK ACK = %d; want rcv.NXT=%d", seg.ACK, issB)
}
}
// TestSYNOnClosingStates verifies SYN rejection in other synchronized states
// beyond ESTABLISHED (FIN-WAIT-1, FIN-WAIT-2, CLOSE-WAIT).
func TestSYNOnClosingStates(t *testing.T) {
states := []State{StateFinWait1, StateFinWait2, StateCloseWait}
for _, state := range states {
t.Run(state.String(), func(t *testing.T) {
const iss, remoteISS, window Size = 100, 300, 1000
var tcb ControlBlock
tcb.HelperInitState(state, Value(iss), Value(iss), window)
tcb.HelperInitRcv(Value(remoteISS), Value(remoteISS), window)
syn := Segment{
SEQ: Value(remoteISS),
Flags: FlagSYN,
WND: 512,
}
err := tcb.Recv(syn)
if err == nil {
t.Fatalf("SYN accepted in %s state; expected rejection", state)
}
if tcb.State() != state {
t.Fatalf("state changed from %s to %s after SYN", state, tcb.State())
}
})
}
}
// TestSYNPreestablished_StillAllowed ensures the fix doesn't break normal SYN
// processing in pre-established states (LISTEN, SYN-SENT, SYN-RCVD).
func TestSYNPreestablished_StillAllowed(t *testing.T) {
const issA, issB, windowA, windowB = 100, 300, 1000, 1000
t.Run("LISTEN", func(t *testing.T) {
var tcb ControlBlock
tcb.HelperInitState(StateListen, issA, issA, windowA)
syn := Segment{SEQ: issB, Flags: FlagSYN, WND: windowB}
err := tcb.Recv(syn)
if err != nil {
t.Fatalf("SYN rejected in LISTEN state: %v", err)
}
if tcb.State() != StateSynRcvd {
t.Fatalf("state = %s; want SYN-RCVD", tcb.State())
}
})
t.Run("SYN-SENT", func(t *testing.T) {
var tcb ControlBlock
tcb.HelperInitState(StateSynSent, issA, issA, windowA)
// Simultaneous open: receive SYN from peer.
syn := Segment{SEQ: issB, Flags: FlagSYN, WND: windowB}
err := tcb.Recv(syn)
if err != nil {
t.Fatalf("SYN rejected in SYN-SENT state: %v", err)
}
if tcb.State() != StateSynRcvd {
t.Fatalf("state = %s; want SYN-RCVD", tcb.State())
}
})
}