From 146f51ce768b9f4f002b78a26178d7896ab0d247 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Thu, 22 Sep 2022 11:08:28 -0700 Subject: [PATCH] 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 --- net/packet/packet.go | 9 +++-- net/packet/packet_test.go | 79 +++++++++++++++++++++++++++++++++------ 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/net/packet/packet.go b/net/packet/packet.go index 063f0ca43..001e469ba 100644 --- a/net/packet/packet.go +++ b/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 diff --git a/net/packet/packet_test.go b/net/packet/packet_test.go index 510808adf..9931e8ceb 100644 --- a/net/packet/packet_test.go +++ b/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 {