net/packet: fix filtering of short IPv4 fragments

The fragment offset is an 8 byte offset rather than a byte offset, so
the short packet limit is now in fragment block size in order to compare
with the offset value.

The packet flags are in the first 3 bits of the flags/frags byte, and
so after conversion to a uint16 little endian value they are at the
start, not the end of the value - the mask for extracting "more
fragments" is adjusted to match this byte.

Extremely short fragments less than 80 bytes are dropped, but fragments
over 80 bytes are now accepted.

Fixes #5727

Signed-off-by: James Tucker <james@tailscale.com>

net/packet/packet.go

@@ -18,7 +18,7 @@ import (
 const unknown = ipproto.Unknown
 
 // RFC1858: prevent overlapping fragment attacks.
-const minFrag = 60 + 20 // max IPv4 header + basic TCP header
+const minFragBlks = (60 + 20) / 8 // max IPv4 header + basic TCP header in fragment blocks (8 bytes each)
 
 type TCPFlag uint8
 
@@ -152,11 +152,12 @@ func (q *Parsed) decode4(b []byte) {
 	// it as Unknown. We can also treat any subsequent fragment that starts
 	// at such a low offset as Unknown.
 	fragFlags := binary.BigEndian.Uint16(b[6:8])
-	moreFrags := (fragFlags & 0x20) != 0
+	moreFrags := (fragFlags & 0x2000) != 0
 	fragOfs := fragFlags & 0x1FFF
+
 	if fragOfs == 0 {
 		// This is the first fragment
-		if moreFrags && len(sub) < minFrag {
+		if moreFrags && len(sub) < minFragBlks {
 			// Suspiciously short first fragment, dump it.
 			q.IPProto = unknown
 			return
@@ -216,7 +217,7 @@ func (q *Parsed) decode4(b []byte) {
 		}
 	} else {
 		// This is a fragment other than the first one.
-		if fragOfs < minFrag {
+		if fragOfs < minFragBlks {
 			// First frag was suspiciously short, so we can't
 			// trust the followup either.
 			q.IPProto = unknown

net/packet/packet_test.go

@@ -37,9 +37,9 @@ func mustIPPort(s string) netip.AddrPort {
 var icmp4RequestBuffer = []byte{
 	// IP header up to checksum
 	0x45, 0x00, 0x00, 0x27, 0xde, 0xad, 0x00, 0x00, 0x40, 0x01, 0x8c, 0x15,
-	// source ip
+	// source IP
 	0x01, 0x02, 0x03, 0x04,
-	// destination ip
+	// destination IP
 	0x05, 0x06, 0x07, 0x08,
 	// ICMP header
 	0x08, 0x00, 0x7d, 0x22,
@@ -61,9 +61,9 @@ var icmp4RequestDecode = Parsed{
 
 var icmp4ReplyBuffer = []byte{
 	0x45, 0x00, 0x00, 0x25, 0x21, 0x52, 0x00, 0x00, 0x40, 0x01, 0x49, 0x73,
-	// source ip
+	// source IP
 	0x05, 0x06, 0x07, 0x08,
-	// destination ip
+	// destination IP
 	0x01, 0x02, 0x03, 0x04,
 	// ICMP header
 	0x00, 0x00, 0xe6, 0x9e,
@@ -119,9 +119,9 @@ var unknownPacketDecode = Parsed{
 var tcp4PacketBuffer = []byte{
 	// IP header up to checksum
 	0x45, 0x00, 0x00, 0x37, 0xde, 0xad, 0x00, 0x00, 0x40, 0x06, 0x49, 0x5f,
-	// source ip
+	// source IP
 	0x01, 0x02, 0x03, 0x04,
-	// destination ip
+	// destination IP
 	0x05, 0x06, 0x07, 0x08,
 	// TCP header with SYN, ACK set
 	0x00, 0x7b, 0x02, 0x37, 0x00, 0x00, 0x12, 0x34, 0x00, 0x00, 0x00, 0x00,
@@ -172,9 +172,9 @@ var tcp6RequestDecode = Parsed{
 var udp4RequestBuffer = []byte{
 	// IP header up to checksum
 	0x45, 0x00, 0x00, 0x2b, 0xde, 0xad, 0x00, 0x00, 0x40, 0x11, 0x8c, 0x01,
-	// source ip
+	// source IP
 	0x01, 0x02, 0x03, 0x04,
-	// destination ip
+	// destination IP
 	0x05, 0x06, 0x07, 0x08,
 	// UDP header
 	0x00, 0x7b, 0x02, 0x37, 0x00, 0x17, 0x72, 0x1d,
@@ -197,9 +197,9 @@ var udp4RequestDecode = Parsed{
 var invalid4RequestBuffer = []byte{
 	// IP header up to checksum. IHL field points beyond end of packet.
 	0x4a, 0x00, 0x00, 0x14, 0xde, 0xad, 0x00, 0x00, 0x40, 0x11, 0x8c, 0x01,
-	// source ip
+	// source IP
 	0x01, 0x02, 0x03, 0x04,
-	// destination ip
+	// destination IP
 	0x05, 0x06, 0x07, 0x08,
 }
 
@@ -244,9 +244,9 @@ var udp6RequestDecode = Parsed{
 var udp4ReplyBuffer = []byte{
 	// IP header up to checksum
 	0x45, 0x00, 0x00, 0x29, 0x21, 0x52, 0x00, 0x00, 0x40, 0x11, 0x49, 0x5f,
-	// source ip
+	// source IP
 	0x05, 0x06, 0x07, 0x08,
-	// destination ip
+	// destination IP
 	0x01, 0x02, 0x03, 0x04,
 	// UDP header
 	0x02, 0x37, 0x00, 0x7b, 0x00, 0x15, 0xd3, 0x9d,
@@ -265,6 +265,59 @@ var udp4ReplyDecode = Parsed{
 	Dst:     mustIPPort("5.6.7.8:123"),
 }
 
+// First TCP fragment of a packet with leading 24 bytes of 'a's
+var tcp4MediumFragmentBuffer = []byte{
+	// IP header up to checksum
+	0x45, 0x20, 0x00, 0x4c, 0x2c, 0x62, 0x20, 0x00, 0x22, 0x06, 0x3a, 0x0f,
+	// source IP
+	0x01, 0x02, 0x03, 0x04,
+	// destination IP
+	0x05, 0x06, 0x07, 0x08,
+	// TCP header
+	0x00, 0x50, 0xf3, 0x8c, 0x58, 0xad, 0x60, 0x94, 0x25, 0xe4, 0x23, 0xa8, 0x80,
+	0x10, 0x01, 0xfd, 0xc6, 0x6e, 0x00, 0x00, 0x01, 0x01, 0x08, 0x0a, 0xff, 0x60,
+	0xfb, 0xfe, 0xba, 0x31, 0x78, 0x6a,
+	// data
+	0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61,
+	0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61,
+}
+
+var tcp4MediumFragmentDecode = Parsed{
+	b:       tcp4MediumFragmentBuffer,
+	subofs:  20,
+	dataofs: 52,
+	length:  len(tcp4MediumFragmentBuffer),
+
+	IPVersion: 4,
+	IPProto:   TCP,
+	Src:       mustIPPort("1.2.3.4:80"),
+	Dst:       mustIPPort("5.6.7.8:62348"),
+	TCPFlags:  0x10,
+}
+
+var tcp4ShortFragmentBuffer = []byte{
+	// IP header up to checksum
+	0x45, 0x20, 0x00, 0x1e, 0x2c, 0x62, 0x20, 0x00, 0x22, 0x06, 0x3c, 0x4f,
+	// source IP
+	0x01, 0x02, 0x03, 0x04,
+	// destination IP
+	0x05, 0x06, 0x07, 0x08,
+	// partial TCP header
+	0x00, 0x50, 0xf3, 0x8c, 0x58, 0xad, 0x60, 0x94, 0x00, 0x00,
+}
+
+var tcp4ShortFragmentDecode = Parsed{
+	b:       tcp4ShortFragmentBuffer,
+	subofs:  20,
+	dataofs: 0,
+	length:  len(tcp4ShortFragmentBuffer),
+	// short fragments are rejected (marked unknown) to avoid header attacks as described in RFC 1858
+	IPProto:   ipproto.Unknown,
+	IPVersion: 4,
+	Src:       mustIPPort("1.2.3.4:0"),
+	Dst:       mustIPPort("5.6.7.8:0"),
+}
+
 var igmpPacketBuffer = []byte{
 	// IP header up to checksum
 	0x46, 0xc0, 0x00, 0x20, 0x00, 0x00, 0x40, 0x00, 0x01, 0x02, 0x41, 0x22,
@@ -404,6 +457,8 @@ func TestDecode(t *testing.T) {
 		{"invalid4", invalid4RequestBuffer, invalid4RequestDecode},
 		{"ipv4_tsmp", ipv4TSMPBuffer, ipv4TSMPDecode},
 		{"ipv4_sctp", sctpBuffer, sctpDecode},
+		{"ipv4_frag", tcp4MediumFragmentBuffer, tcp4MediumFragmentDecode},
+		{"ipv4_fragtooshort", tcp4ShortFragmentBuffer, tcp4ShortFragmentDecode},
 	}
 
 	for _, tt := range tests {