From 3514c5e3c7c59c697eab82ade3b928883adc4c34 Mon Sep 17 00:00:00 2001 From: fako1024 <> Date: Mon, 27 Apr 2026 13:01:23 +0200 Subject: [PATCH] [bugfix] Guard packet length and snaplen parameters against misuse / overflows --- capture/pcap/pcap.go | 25 +++++-- capture/pcap/pcap_test.go | 136 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 5 deletions(-) diff --git a/capture/pcap/pcap.go b/capture/pcap/pcap.go index a27c76f..bce0517 100644 --- a/capture/pcap/pcap.go +++ b/capture/pcap/pcap.go @@ -21,6 +21,8 @@ import ( "github.com/fako1024/slimcap/capture" ) +const maxSnapLen uint32 = 16 << 20 + // Source denotes a pcap file capture source type Source struct { reader *bufio.Reader @@ -72,6 +74,9 @@ func NewSource(iface string, r io.Reader) (*Source, error) { if obj.header.MagicNumber != MagicNativeEndianess { return nil, fmt.Errorf("invalid pcap header magic: %x", obj.header.MagicNumber) } + if obj.header.Snaplen == 0 || obj.header.Snaplen > maxSnapLen { + return nil, fmt.Errorf("invalid pcap header snaplen: %d (max %d)", obj.header.Snaplen, maxSnapLen) + } // Populate (fake) link information obj.link = &link.Link{ @@ -231,14 +236,24 @@ func (s *Source) nextPacket() (pktHeader PacketHeader, err error) { if s.swapEndianess { pktHeader = pktHeader.SwapEndianess() } + if pktHeader.CaptureLen < 0 { + err = fmt.Errorf("invalid packet capture length: %d", pktHeader.CaptureLen) + return + } - if err = s.nextPacketData(int(pktHeader.CaptureLen)); err == nil { - s.nPackets++ + captureLen := uint32(pktHeader.CaptureLen) + if captureLen > s.header.Snaplen { + err = fmt.Errorf("invalid packet capture length: %d exceeds snaplen %d", pktHeader.CaptureLen, s.header.Snaplen) + return } - // If a callback function was provided, execute it - if s.packetAddCallbackFn != nil { - s.packetAddCallbackFn(s.buf, uint32(pktHeader.OriginalLen), capture.PacketUnknown, s.ipLayerOffset) + if err = s.nextPacketData(int(captureLen)); err == nil { + s.nPackets++ + + // If a callback function was provided, execute it + if s.packetAddCallbackFn != nil { + s.packetAddCallbackFn(s.buf, uint32(pktHeader.OriginalLen), capture.PacketUnknown, s.ipLayerOffset) + } } return diff --git a/capture/pcap/pcap_test.go b/capture/pcap/pcap_test.go index 53d4b01..63dbfd2 100644 --- a/capture/pcap/pcap_test.go +++ b/capture/pcap/pcap_test.go @@ -3,6 +3,7 @@ package pcap import ( "bytes" "embed" + "encoding/binary" "io" "io/fs" "os" @@ -54,6 +55,18 @@ func TestInvalidInput(t *testing.T) { require.EqualError(t, err, "invalid pcap header magic: 0") require.Nil(t, src) }) + + t.Run("invalid snaplen zero", func(t *testing.T) { + src, err := NewSource("pcap", bytes.NewBuffer(buildTestPCAP(t, 0))) + require.EqualError(t, err, "invalid pcap header snaplen: 0 (max 16777216)") + require.Nil(t, src) + }) + + t.Run("invalid snaplen too large", func(t *testing.T) { + src, err := NewSource("pcap", bytes.NewBuffer(buildTestPCAP(t, maxSnapLen+1))) + require.EqualError(t, err, "invalid pcap header snaplen: 16777217 (max 16777216)") + require.Nil(t, src) + }) } func TestReader(t *testing.T) { @@ -203,6 +216,83 @@ func TestCaptureMethods(t *testing.T) { }) } +func TestMalformedPacketCaptureLen(t *testing.T) { + + t.Run("negative capture length", func(t *testing.T) { + src, err := NewSource("pcap", bytes.NewReader(buildTestPCAP(t, 64, + testPacketRecord{captureLen: -1, originalLen: 64}, + ))) + require.Nil(t, err) + require.NotNil(t, src) + defer func() { + require.Nil(t, src.Close()) + }() + + require.NotPanics(t, func() { + payload, pktType, totalLen, err := src.NextPayload(nil) + require.ErrorContains(t, err, "invalid packet capture length: -1") + require.Nil(t, payload) + require.Equal(t, capture.PacketUnknown, pktType) + require.Zero(t, totalLen) + }) + }) + + t.Run("capture length exceeds snaplen", func(t *testing.T) { + src, err := NewSource("pcap", bytes.NewReader(buildTestPCAP(t, 64, + testPacketRecord{captureLen: 65, originalLen: 65}, + ))) + require.Nil(t, err) + require.NotNil(t, src) + defer func() { + require.Nil(t, src.Close()) + }() + + require.NotPanics(t, func() { + payload, pktType, totalLen, err := src.NextPayload(nil) + require.ErrorContains(t, err, "invalid packet capture length: 65 exceeds snaplen 64") + require.Nil(t, payload) + require.Equal(t, capture.PacketUnknown, pktType) + require.Zero(t, totalLen) + }) + }) +} + +func TestPacketAddCallbackFnCalledOnReadSuccessOnly(t *testing.T) { + + src, err := NewSource("pcap", bytes.NewReader(buildTestPCAP(t, 64, + testPacketRecord{captureLen: 4, originalLen: 4, payload: []byte{1, 2, 3, 4}}, + testPacketRecord{captureLen: -1, originalLen: 4}, + ))) + require.Nil(t, err) + require.NotNil(t, src) + defer func() { + require.Nil(t, src.Close()) + }() + + callbackCount := 0 + var callbackPayload []byte + src.PacketAddCallbackFn(func(payload []byte, totalLen uint32, pktType, ipLayerOffset byte) { + callbackCount++ + callbackPayload = append([]byte(nil), payload...) + }) + + payload, pktType, totalLen, err := src.NextPayload(nil) + require.Nil(t, err) + require.Equal(t, []byte{1, 2, 3, 4}, payload) + require.Equal(t, capture.PacketUnknown, pktType) + require.Equal(t, uint32(4), totalLen) + require.Equal(t, 1, callbackCount) + require.Equal(t, []byte{1, 2, 3, 4}, callbackPayload) + + payload, pktType, totalLen, err = src.NextPayload(nil) + require.ErrorContains(t, err, "invalid packet capture length: -1") + require.Nil(t, payload) + require.Equal(t, capture.PacketUnknown, pktType) + require.Zero(t, totalLen) + require.Equal(t, 1, callbackCount) + require.Equal(t, []byte{1, 2, 3, 4}, callbackPayload) +} + func testCaptureMethods(t *testing.T, fn func(t *testing.T, src *Source)) { for _, dirent := range testData { file, err := os.Open(filepath.Join(testDataPath, dirent.Name())) @@ -222,6 +312,52 @@ func testCaptureMethods(t *testing.T, fn func(t *testing.T, src *Source)) { } } +type testPacketRecord struct { + captureLen int32 + originalLen int32 + payload []byte +} + +func buildTestPCAP(t *testing.T, snapLen uint32, packets ...testPacketRecord) []byte { + t.Helper() + + var out bytes.Buffer + write := func(v interface{}) { + require.Nil(t, binary.Write(&out, binary.LittleEndian, v)) + } + + write(uint32(MagicNativeEndianess)) + write(uint16(2)) + write(uint16(4)) + write(int32(0)) + write(uint32(0)) + write(snapLen) + write(uint32(link.TypeEthernet)) + + for _, packet := range packets { + write(int32(0)) + write(int32(0)) + write(packet.captureLen) + write(packet.originalLen) + + if packet.captureLen > 0 { + captureLen := int(packet.captureLen) + payload := packet.payload + if len(payload) < captureLen { + payload = make([]byte, captureLen) + copy(payload, packet.payload) + } else { + payload = payload[:captureLen] + } + + _, err := out.Write(payload) + require.Nil(t, err) + } + } + + return out.Bytes() +} + //go:embed testdata/* var pcaps embed.FS