[bugfix] Guard packet length and snaplen parameters against misuse / overflows

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

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