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) + }) }