Repository: pion/webrtc (go)

Issue:
nil pointer dereference in RTPReceiver readRTP
<!--
  Come join our community on Slack https://pion.ly/slack for quicker support and more resources

  FAQ of common issues and questions https://github.com/pion/webrtc/wiki/FAQ

  Free book that covers general WebRTC https://webrtcforthecurious.com/

  Please answer these questions before submitting your issue. Thanks!
-->

### Your environment.
  - Version: *v3.2.40*
```
[signal 0xc0000005 code=0x0 addr=0x18 pc=0x7ff724f803d7]

goroutine 129 [running]:
github.com/pion/webrtc/v3.(*RTPReceiver).readRTP(0xc000414d80, {0xc000150600, 0x5b4, 0x5b4}, 0xc00077ea20)
        C:/Users/teng.lee/go/pkg/mod/github.com/pion/webrtc/v3@v3.2.40/rtpreceiver.go:369 +0xf7
github.com/pion/webrtc/v3.(*TrackRemote).Read(0xc00077ea20, {0xc000150600, 0x5b4, 0x5b4})
        C:/Users/teng.lee/go/pkg/mod/github.com/pion/webrtc/v3@v3.2.40/track_remote.go:139 +0x24e
github.com/pion/webrtc/v3.(*TrackRemote).peek(0xc00077ea20, {0xc000150600, 0x16?, 0x5b4})
        C:/Users/teng.lee/go/pkg/mod/github.com/pion/webrtc/v3@v3.2.40/track_remote.go:191 +0x26
github.com/pion/webrtc/v3.(*PeerConnection).startReceiver.func1(0xc00077ea20)
        C:/Users/teng.lee/go/pkg/mod/github.com/pion/webrtc/v3@v3.2.40/peerconnection.go:1241 +0x7a
created by github.com/pion/webrtc/v3.(*PeerConnection).startReceiver in goroutine 105
        C:/Users/teng.lee/go/pkg/mod/github.com/pion/webrtc/v3@v3.2.40/peerconnection.go:1239 +0x1e5
exit status 2
```

### What did you do?

Run pion with unsupported codec at remote? I'm not sure how to pin point the problem

### What did you expect?

ignore error and continue running

### What happened?

nil pointer dereference and panic

### Temporary solution

Right now, I added a check for rtcpinterceptor to be nil before returning, this is not ideal as I need to modify the source code

```
// readRTP should only be called by a track, this only exists so we can keep state in one place
func (r *RTPReceiver) readRTP(b []byte, reader *TrackRemote) (n int, a interceptor.Attributes, err error) {
	<-r.received
	if t := r.streamsForTrack(reader); t != nil {
		if t.rtcpInterceptor == nil {
			return 0, nil, nil
		}
		return t.rtpInterceptor.Read(b, a)
	}

	return 0, nil, fmt.Errorf("%w: %d", errRTPReceiverWithSSRCTrackStreamNotFound, reader.SSRC())
}
```

Pull request:
fix: rtcp interceptor nil panic
#### Description

Add a nil rtpinterceptor check before invoking read method

#### Reference issue

Fixes #2929


Patch:
diff --git a/rtpreceiver.go b/rtpreceiver.go
index b4757636..109f88a4 100644
--- a/rtpreceiver.go
+++ b/rtpreceiver.go
@@ -193,7 +193,6 @@ func (r *RTPReceiver) startReceive(parameters RTPReceiveParameters) error { //no
 		return errRTPReceiverReceiveAlreadyCalled
 	default:
 	}
-	defer close(r.received)
 
 	globalParams := r.getParameters()
 	codec := RTPCodecCapability{}
@@ -257,6 +256,8 @@ func (r *RTPReceiver) startReceive(parameters RTPReceiveParameters) error { //no
 		}
 	}
 
+	close(r.received)
+
 	return nil
 }
 
@@ -404,7 +405,12 @@ func (r *RTPReceiver) streamsForTrack(t *TrackRemote) *trackStreams {
 
 // readRTP should only be called by a track, this only exists so we can keep state in one place.
 func (r *RTPReceiver) readRTP(b []byte, reader *TrackRemote) (n int, a interceptor.Attributes, err error) {
-	<-r.received
+	select {
+	case <-r.received:
+	case <-r.closed:
+		return 0, nil, io.EOF
+	}
+
 	if t := r.streamsForTrack(reader); t != nil {
 		return t.rtpInterceptor.Read(b, a)
 	}


Test patch:
diff --git a/peerconnection_renegotiation_test.go b/peerconnection_renegotiation_test.go
index 2782e953..fccb0299 100644
--- a/peerconnection_renegotiation_test.go
+++ b/peerconnection_renegotiation_test.go
@@ -1127,10 +1127,13 @@ func TestPeerConnection_Renegotiation_Simulcast(t *testing.T) {
 		defer trackMapLock.Unlock()
 
 		for _, track := range trackMap {
-			_, _, err := track.ReadRTP() // Ignore first Read, this is our peeked data
-			assert.Nil(t, err)
+			_, _, err := track.ReadRTP()
+
+			// Ignore first Read, this was our peeked data
+			if err == nil {
+				_, _, err = track.ReadRTP()
+			}
 
-			_, _, err = track.ReadRTP()
 			assert.Equal(t, err, io.EOF)
 		}
 	}
diff --git a/rtpreceiver_go_test.go b/rtpreceiver_go_test.go
index 2b7ff6de..81e7d7ad 100644
--- a/rtpreceiver_go_test.go
+++ b/rtpreceiver_go_test.go
@@ -8,6 +8,7 @@ package webrtc
 
 import (
 	"context"
+	"io"
 	"testing"
 	"time"
 
@@ -74,3 +75,38 @@ func TestSetRTPParameters(t *testing.T) {
 	assert.NoError(t, wan.Stop())
 	closePairNow(t, sender, receiver)
 }
+
+func TestReceiveError(t *testing.T) {
+	api := NewAPI()
+
+	dtlsTransport, err := api.NewDTLSTransport(nil, nil)
+	assert.NoError(t, err)
+
+	rtpReceiver, err := api.NewRTPReceiver(RTPCodecTypeVideo, dtlsTransport)
+	assert.NoError(t, err)
+
+	rtpParameters := RTPReceiveParameters{
+		Encodings: []RTPDecodingParameters{
+			{
+				RTPCodingParameters: RTPCodingParameters{
+					SSRC: 1000,
+				},
+			},
+		},
+	}
+
+	assert.Error(t, rtpReceiver.Receive(rtpParameters))
+
+	chanErrs := make(chan error)
+	go func() {
+		_, _, chanErr := rtpReceiver.Read(nil)
+		chanErrs <- chanErr
+
+		_, _, chanErr = rtpReceiver.Track().ReadRTP()
+		chanErrs <- chanErr
+	}()
+
+	assert.NoError(t, rtpReceiver.Stop())
+	assert.Error(t, io.ErrClosedPipe, <-chanErrs)
+	assert.Error(t, io.ErrClosedPipe, <-chanErrs)
+}