From 442a52879b6b19f5b455d96a3b2a3f1e3fe57649 Mon Sep 17 00:00:00 2001 From: brad-defined <77982333+brad-defined@users.noreply.github.com> Date: Wed, 11 Jun 2025 15:15:15 -0400 Subject: [PATCH] Fix off by one error in IPv6 packet parser (#1419) --- outside.go | 9 ++++----- outside_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/outside.go b/outside.go index 3a7b3a7..6d4127d 100644 --- a/outside.go +++ b/outside.go @@ -312,12 +312,11 @@ func parseV6(data []byte, incoming bool, fp *firewall.Packet) error { offset := ipv6.HeaderLen // Start at the end of the ipv6 header next := 0 for { - if dataLen < offset { + if protoAt >= dataLen { break } - proto := layers.IPProtocol(data[protoAt]) - //fmt.Println(proto, protoAt) + switch proto { case layers.IPProtocolICMPv6, layers.IPProtocolESP, layers.IPProtocolNoNextHeader: fp.Protocol = uint8(proto) @@ -365,7 +364,7 @@ func parseV6(data []byte, incoming bool, fp *firewall.Packet) error { case layers.IPProtocolAH: // Auth headers, used by IPSec, have a different meaning for header length - if dataLen < offset+1 { + if dataLen <= offset+1 { break } @@ -373,7 +372,7 @@ func parseV6(data []byte, incoming bool, fp *firewall.Packet) error { default: // Normal ipv6 header length processing - if dataLen < offset+1 { + if dataLen <= offset+1 { break } diff --git a/outside_test.go b/outside_test.go index c63e57d..38dbef6 100644 --- a/outside_test.go +++ b/outside_test.go @@ -117,6 +117,45 @@ func Test_newPacket_v6(t *testing.T) { err = newPacket(buffer.Bytes(), true, p) require.ErrorIs(t, err, ErrIPv6CouldNotFindPayload) + // A v6 packet with a hop-by-hop extension + // ICMPv6 Payload (Echo Request) + icmpLayer := layers.ICMPv6{ + TypeCode: layers.ICMPv6TypeEchoRequest, + } + // Hop-by-Hop Extension Header + hopOption := layers.IPv6HopByHopOption{} + hopOption.OptionData = []byte{0, 0, 0, 0} + hopByHop := layers.IPv6HopByHop{} + hopByHop.Options = append(hopByHop.Options, &hopOption) + + ip = layers.IPv6{ + Version: 6, + HopLimit: 128, + NextHeader: layers.IPProtocolIPv6Destination, + SrcIP: net.IPv6linklocalallrouters, + DstIP: net.IPv6linklocalallnodes, + } + + buffer.Clear() + err = gopacket.SerializeLayers(buffer, gopacket.SerializeOptions{ + ComputeChecksums: false, + FixLengths: true, + }, &ip, &hopByHop, &icmpLayer) + if err != nil { + panic(err) + } + // Ensure buffer length checks during parsing with the next 2 tests. + + // A full IPv6 header and 1 byte in the first extension, but missing + // the length byte. + err = newPacket(buffer.Bytes()[:41], true, p) + require.ErrorIs(t, err, ErrIPv6CouldNotFindPayload) + + // A full IPv6 header plus 1 full extension, but only 1 byte of the + // next layer, missing length byte + err = newPacket(buffer.Bytes()[:49], true, p) + require.ErrorIs(t, err, ErrIPv6CouldNotFindPayload) + // A good ICMP packet ip = layers.IPv6{ Version: 6, @@ -288,6 +327,10 @@ func Test_newPacket_v6(t *testing.T) { assert.Equal(t, uint16(22), p.LocalPort) assert.False(t, p.Fragment) + // Ensure buffer bounds checking during processing + err = newPacket(b[:41], true, p) + require.ErrorIs(t, err, ErrIPv6PacketTooShort) + // Invalid AH header b = buffer.Bytes() err = newPacket(b, true, p)