From c4c356f6cafb8e483297bdc47b97e0547e6deef6 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Tue, 11 Aug 2026 18:25:10 +0530 Subject: [PATCH] Cover a couple of more cases on data track runt packet handling. (#4741) * Cover a couple of more cases on data track runt packet handling. * Guard data track header parser against extensions-size integer wraparound. Widen the extensions-size arithmetic to int so a 0xFFFF wire value no longer wraps in uint16, and reject any packet whose computed hdrSize exceeds the buffer before slicing the payload. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- pkg/rtc/datatrack/packet.go | 15 +++++++++++--- pkg/rtc/datatrack/packet_test.go | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/pkg/rtc/datatrack/packet.go b/pkg/rtc/datatrack/packet.go index 487a1c6c9..42fe1eef2 100644 --- a/pkg/rtc/datatrack/packet.go +++ b/pkg/rtc/datatrack/packet.go @@ -126,11 +126,14 @@ func (h *Header) Unmarshal(buf []byte) (int, error) { h.Timestamp = binary.BigEndian.Uint32(buf[timestampOffset : timestampOffset+timestampLength]) if h.HasExtensions { - extensionsSize := (binary.BigEndian.Uint16(buf[extensionsSizeOffset:extensionsSizeOffset+extensionsSizeLength])+1)*4 - extensionsSizeLength + if len(buf) < extensionsSizeOffset+extensionsSizeLength { + return 0, fmt.Errorf("%w: %d < %d", errHeaderSizeInsufficient, len(buf), extensionsSizeOffset+extensionsSizeLength) + } + extensionsSize := (int(binary.BigEndian.Uint16(buf[extensionsSizeOffset:extensionsSizeOffset+extensionsSizeLength]))+1)*4 - extensionsSizeLength hdrSize += extensionsSizeLength extensionHeaderSize := extensionIDLength + extensionSizeLength - remainingSize := int(extensionsSize) + remainingSize := extensionsSize idx := extensionsSizeOffset + extensionsSizeLength for remainingSize != 0 { // read extension header @@ -140,6 +143,9 @@ func (h *Header) Unmarshal(buf []byte) (int, error) { id := buf[idx] if id == 0 { // end of extensions, padding has started + if len(buf[idx:]) < remainingSize { + return 0, fmt.Errorf("%w: %d/%d < %d", errExtensionSizeInsufficient, remainingSize, len(buf[idx:]), remainingSize) + } hdrSize += remainingSize break } @@ -163,7 +169,7 @@ func (h *Header) Unmarshal(buf []byte) (int, error) { idx += size hdrSize += size } - h.ExtensionsSize = extensionsSize - uint16(remainingSize) + h.ExtensionsSize = uint16(extensionsSize - remainingSize) } return hdrSize, nil @@ -271,6 +277,9 @@ func (p *Packet) Unmarshal(buf []byte) error { if err != nil { return err } + if hdrSize > len(buf) { + return fmt.Errorf("%w: %d < %d", errBufferSizeInsufficient, len(buf), hdrSize) + } p.Payload = buf[hdrSize:] return nil diff --git a/pkg/rtc/datatrack/packet_test.go b/pkg/rtc/datatrack/packet_test.go index 4d8b2c6cf..8f1bc672b 100644 --- a/pkg/rtc/datatrack/packet_test.go +++ b/pkg/rtc/datatrack/packet_test.go @@ -256,4 +256,39 @@ func TestPacket(t *testing.T) { err = unmarshaled.Unmarshal(badPacket) require.Error(t, err) }) + + t.Run("oversized extension padding does not panic", func(t *testing.T) { + var unmarshaled Packet + // HasExtensions set, extensionsSize describes more bytes than present, + // terminated by a 0x00 padding id -> hdrSize would exceed len(buf) + badPacket := []byte{ + 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + } + err := unmarshaled.Unmarshal(badPacket) + require.Error(t, err) + }) + + t.Run("extensions size wraparound does not panic", func(t *testing.T) { + var unmarshaled Packet + // 0xFFFF extensions-size field wraps (raw+1)*4 uint16 arithmetic to a huge + // remainingSize; the 0x00 padding id must not push hdrSize past len(buf) + badPacket := []byte{ + 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0x00, + } + err := unmarshaled.Unmarshal(badPacket) + require.Error(t, err) + }) + + t.Run("truncated extensions size field does not panic", func(t *testing.T) { + var unmarshaled Packet + // HasExtensions set but buffer too short to hold the extensionsSize field + badPacket := []byte{ + 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, + } + err := unmarshaled.Unmarshal(badPacket) + require.Error(t, err) + }) }