Fix padding overflow with PacketFactory

RTP padding overflow is now handled, instead of catching a panic.
pion/webrtc#3148

Author: JoTurk

internal/rtpbuffer/errors.go

@@ -12,4 +12,5 @@ var (
 	errPacketReleased          = errors.New("could not retain packet, already released")
 	errFailedToCastHeaderPool  = errors.New("could not access header pool, failed cast")
 	errFailedToCastPayloadPool = errors.New("could not access payload pool, failed cast")
+	errPaddingOverflow         = errors.New("padding size exceeds payload size")
 )

internal/rtpbuffer/packet_factory.go

@@ -84,7 +84,7 @@ func (m *PacketFactoryCopy) NewPacket(
 		}
 	}
 
-	if rtxSsrc != 0 && rtxPayloadType != 0 {
+	if rtxSsrc != 0 && rtxPayloadType != 0 { //nolint:nestif
 		if payload == nil {
 			retainablePacket.buffer, ok = m.payloadPool.Get().(*[]byte)
 			if !ok {
@@ -105,6 +105,11 @@ func (m *PacketFactoryCopy) NewPacket(
 		if retainablePacket.header.Padding && retainablePacket.payload != nil && len(retainablePacket.payload) > 0 {
 			paddingLength := int(retainablePacket.payload[len(retainablePacket.payload)-1])
 			retainablePacket.header.Padding = false
+
+			if paddingLength > len(retainablePacket.payload) {
+				return nil, errPaddingOverflow
+			}
+
 			retainablePacket.payload = (*retainablePacket.buffer)[:len(retainablePacket.payload)-paddingLength]
 		}
 	}

internal/rtpbuffer/rtpbuffer_test.go

@@ -4,6 +4,7 @@
 package rtpbuffer
 
 import (
+	"bytes"
 	"testing"
 
 	"github.com/pion/rtp"
@@ -218,3 +219,50 @@ func TestRTPBuffer_Overridden_WithRTX_NILPayload(t *testing.T) {
 
 	require.Nil(t, sb.Get(1))
 }
+
+func TestRTPBuffer_Padding(t *testing.T) {
+	pm := NewPacketFactoryCopy()
+	sb, err := NewRTPBuffer(1)
+	require.NoError(t, err)
+	require.Equal(t, uint16(1), sb.size)
+
+	t.Run("valid padding is stripped", func(t *testing.T) {
+		origPayload := []byte{116, 101, 115, 116}
+		expected := []byte{0, 1, 116, 101, 115, 116}
+
+		padLen := 120
+		padded := make([]byte, 0)
+		padded = append(padded, origPayload...)
+		padded = append(padded, bytes.Repeat([]byte{0}, padLen-1)...)
+		padded = append(padded, byte(padLen))
+
+		pkt, err := pm.NewPacket(&rtp.Header{
+			SequenceNumber: 1,
+			Padding:        true,
+		}, padded, 1, 1)
+		require.NoError(t, err)
+
+		sb.Add(pkt)
+
+		retrieved := sb.Get(1)
+		require.NotNil(t, retrieved)
+		defer retrieved.Release()
+
+		require.False(t, retrieved.Header().Padding, "P-bit should be cleared after trimming")
+
+		actual := retrieved.Payload()
+		require.Equal(t, len(expected), len(actual), "payload length after trimming")
+		require.Equal(t, expected, actual, "payload content after trimming")
+	})
+
+	t.Run("overflow padding returns io.ErrShortBuffer", func(t *testing.T) {
+		overflow := []byte{0, 1, 200}
+
+		_, err := pm.NewPacket(&rtp.Header{
+			SequenceNumber: 2,
+			Padding:        true,
+		}, overflow, 1, 1)
+
+		require.ErrorIs(t, err, errPaddingOverflow, "factory should reject invalid padding")
+	})
+}