forbid DecodeUntilMarker() from reusing buffers to avoid race conditions (#284)

This commit is contained in:
Alessandro Ros
2023-05-16 16:58:16 +02:00
committed by GitHub
parent 5a543b71ed
commit f94885005f
7 changed files with 117 additions and 24 deletions

View File

@@ -6,7 +6,7 @@
[![CodeCov](https://codecov.io/gh/bluenviron/gortsplib/branch/main/graph/badge.svg)](https://app.codecov.io/gh/bluenviron/gortsplib/branch/main)
[![PkgGoDev](https://pkg.go.dev/badge/github.com/bluenviron/gortsplib/v3)](https://pkg.go.dev/github.com/bluenviron/gortsplib/v3#pkg-index)
RTSP 1.0 client and server library for the Go programming language, written for [MediaMTX](https://github.com/aler9/mediamtx).
RTSP 1.0 client and server library for the Go programming language, written for [MediaMTX](https://github.com/bluenviron/mediamtx).
Go ≥ 1.18 is required.
@@ -97,7 +97,6 @@ https://pkg.go.dev/github.com/bluenviron/gortsplib/v3#pkg-index
## Standards
* [Codec standards](https://github.com/bluenviron/mediacommon#standards)
* [RFC2326, RTSP 1.0](https://datatracker.ietf.org/doc/html/rfc2326)
* [RFC7826, RTSP 2.0](https://datatracker.ietf.org/doc/html/rfc7826)
* [RFC8866, SDP: Session Description Protocol](https://datatracker.ietf.org/doc/html/rfc8866)
@@ -114,13 +113,14 @@ https://pkg.go.dev/github.com/bluenviron/gortsplib/v3#pkg-index
* [RFC7587, RTP Payload Format for the Opus Speech and Audio Codec](https://datatracker.ietf.org/doc/html/rfc7587)
* [RFC3640, RTP Payload Format for Transport of MPEG-4 Elementary Streams](https://datatracker.ietf.org/doc/html/rfc3640)
* [RTP Payload Format For AV1 (v1.0)](https://aomediacodec.github.io/av1-rtp-spec/)
* [Codec standards](https://github.com/bluenviron/mediacommon#standards)
* [Golang project layout](https://github.com/golang-standards/project-layout)
## Links
Related projects
* [MediaMTX](https://github.com/aler9/mediamtx)
* [MediaMTX](https://github.com/bluenviron/mediamtx)
* [pion/sdp (SDP library used internally)](https://github.com/pion/sdp)
* [pion/rtp (RTP library used internally)](https://github.com/pion/rtp)
* [pion/rtcp (RTCP library used internally)](https://github.com/pion/rtcp)

View File

@@ -40,7 +40,8 @@ type Decoder struct {
fragments [][]byte
// for DecodeUntilMarker()
obuBuffer [][]byte
frameBuffer [][]byte
frameBufferLen int
}
// Init initializes the decoder.
@@ -131,20 +132,27 @@ func (d *Decoder) DecodeUntilMarker(pkt *rtp.Packet) ([][]byte, time.Duration, e
if err != nil {
return nil, 0, err
}
l := len(obus)
if (len(d.obuBuffer) + len(obus)) > av1.MaxOBUsPerTemporalUnit {
return nil, 0, fmt.Errorf("OBU count (%d) exceeds maximum allowed (%d)",
len(d.obuBuffer)+len(obus), av1.MaxOBUsPerTemporalUnit)
if (d.frameBufferLen + l) > av1.MaxOBUsPerTemporalUnit {
d.frameBuffer = nil
d.frameBufferLen = 0
return nil, 0, fmt.Errorf("OBU count exceeds maximum allowed (%d)",
av1.MaxOBUsPerTemporalUnit)
}
d.obuBuffer = append(d.obuBuffer, obus...)
d.frameBuffer = append(d.frameBuffer, obus...)
d.frameBufferLen += l
if !pkt.Marker {
return nil, 0, ErrMorePacketsNeeded
}
ret := d.obuBuffer
d.obuBuffer = d.obuBuffer[:0]
ret := d.frameBuffer
// do not reuse frameBuffer to avoid race conditions
d.frameBuffer = nil
d.frameBufferLen = 0
return ret, pts, nil
}

View File

@@ -3,6 +3,7 @@ package rtpav1
import (
"testing"
"github.com/bluenviron/mediacommon/pkg/codecs/av1"
"github.com/pion/rtp"
"github.com/stretchr/testify/require"
)
@@ -30,6 +31,28 @@ func TestDecode(t *testing.T) {
}
}
func TestDecoderErrorLimit(t *testing.T) {
d := &Decoder{}
d.Init()
var err error
for i := 0; i <= av1.MaxOBUsPerTemporalUnit; i++ {
_, _, err = d.DecodeUntilMarker(&rtp.Packet{
Header: rtp.Header{
Version: 2,
Marker: false,
PayloadType: 96,
SequenceNumber: 17645,
Timestamp: 2289527317,
SSRC: 0x9dbb7812,
},
Payload: []byte{1, 2, 3, 4},
})
}
require.EqualError(t, err, "OBU count exceeds maximum allowed (10)")
}
func FuzzDecoder(f *testing.F) {
f.Fuzz(func(t *testing.T, a []byte, am bool, b []byte, bm bool) {
d := &Decoder{}

View File

@@ -44,7 +44,8 @@ type Decoder struct {
annexBMode bool
// for DecodeUntilMarker()
naluBuffer [][]byte
frameBuffer [][]byte
frameBufferLen int
}
// Init initializes the decoder.
@@ -175,20 +176,27 @@ func (d *Decoder) DecodeUntilMarker(pkt *rtp.Packet) ([][]byte, time.Duration, e
if err != nil {
return nil, 0, err
}
l := len(nalus)
if (len(d.naluBuffer) + len(nalus)) > h264.MaxNALUsPerGroup {
return nil, 0, fmt.Errorf("NALU count (%d) exceeds maximum allowed (%d)",
len(d.naluBuffer)+len(nalus), h264.MaxNALUsPerGroup)
if (d.frameBufferLen + l) > h264.MaxNALUsPerGroup {
d.frameBuffer = nil
d.frameBufferLen = 0
return nil, 0, fmt.Errorf("NALU count exceeds maximum allowed (%d)",
h264.MaxNALUsPerGroup)
}
d.naluBuffer = append(d.naluBuffer, nalus...)
d.frameBuffer = append(d.frameBuffer, nalus...)
d.frameBufferLen += l
if !pkt.Marker {
return nil, 0, ErrMorePacketsNeeded
}
ret := d.naluBuffer
d.naluBuffer = d.naluBuffer[:0]
ret := d.frameBuffer
// do not reuse frameBuffer to avoid race conditions
d.frameBuffer = nil
d.frameBufferLen = 0
return ret, pts, nil
}

View File

@@ -4,6 +4,7 @@ import (
"bytes"
"testing"
"github.com/bluenviron/mediacommon/pkg/codecs/h264"
"github.com/pion/rtp"
"github.com/stretchr/testify/require"
)
@@ -183,6 +184,28 @@ func TestDecodeUntilMarker(t *testing.T) {
require.Equal(t, [][]byte{{0x01, 0x02}, {0x01, 0x02}}, nalus)
}
func TestDecoderErrorLimit(t *testing.T) {
d := &Decoder{}
d.Init()
var err error
for i := 0; i <= h264.MaxNALUsPerGroup; i++ {
_, _, err = d.DecodeUntilMarker(&rtp.Packet{
Header: rtp.Header{
Version: 2,
Marker: false,
PayloadType: 96,
SequenceNumber: 17645,
Timestamp: 2289527317,
SSRC: 0x9dbb7812,
},
Payload: []byte{1, 2, 3, 4},
})
}
require.EqualError(t, err, "NALU count exceeds maximum allowed (20)")
}
func FuzzDecoder(f *testing.F) {
f.Fuzz(func(t *testing.T, a []byte, b []byte) {
d := &Decoder{}

View File

@@ -42,7 +42,8 @@ type Decoder struct {
fragments [][]byte
// for DecodeUntilMarker()
naluBuffer [][]byte
frameBuffer [][]byte
frameBufferLen int
}
// Init initializes the decoder.
@@ -167,20 +168,27 @@ func (d *Decoder) DecodeUntilMarker(pkt *rtp.Packet) ([][]byte, time.Duration, e
if err != nil {
return nil, 0, err
}
l := len(nalus)
if (len(d.naluBuffer) + len(nalus)) > h265.MaxNALUsPerGroup {
return nil, 0, fmt.Errorf("NALU count (%d) exceeds maximum allowed (%d)",
len(d.naluBuffer)+len(nalus), h265.MaxNALUsPerGroup)
if (d.frameBufferLen + l) > h265.MaxNALUsPerGroup {
d.frameBuffer = nil
d.frameBufferLen = 0
return nil, 0, fmt.Errorf("NALU count exceeds maximum allowed (%d)",
h265.MaxNALUsPerGroup)
}
d.naluBuffer = append(d.naluBuffer, nalus...)
d.frameBuffer = append(d.frameBuffer, nalus...)
d.frameBufferLen += l
if !pkt.Marker {
return nil, 0, ErrMorePacketsNeeded
}
ret := d.naluBuffer
d.naluBuffer = d.naluBuffer[:0]
ret := d.frameBuffer
// do not reuse frameBuffer to avoid race conditions
d.frameBuffer = nil
d.frameBufferLen = 0
return ret, pts, nil
}

View File

@@ -3,6 +3,7 @@ package rtph265
import (
"testing"
"github.com/bluenviron/mediacommon/pkg/codecs/h265"
"github.com/pion/rtp"
"github.com/stretchr/testify/require"
)
@@ -35,6 +36,28 @@ func TestDecode(t *testing.T) {
}
}
func TestDecoderErrorLimit(t *testing.T) {
d := &Decoder{}
d.Init()
var err error
for i := 0; i <= h265.MaxNALUsPerGroup; i++ {
_, _, err = d.DecodeUntilMarker(&rtp.Packet{
Header: rtp.Header{
Version: 2,
Marker: false,
PayloadType: 96,
SequenceNumber: 17645,
Timestamp: 2289527317,
SSRC: 0x9dbb7812,
},
Payload: []byte{1, 2, 3, 4},
})
}
require.EqualError(t, err, "NALU count exceeds maximum allowed (20)")
}
func FuzzDecoder(f *testing.F) {
f.Fuzz(func(t *testing.T, a []byte, b []byte) {
d := &Decoder{}