Repository: pion/rtp
Pull request #205 (2023-01-07 16:36:45)

Fix multiple crashes when using VP9 depacketizer
#### Description

Multiple optional fields of the VP9 RTP header are read regardless of the buffer size. This can easily lead to multiple crashes. This PR add the necessary checks in order to avoid crashes.

#### Reference issue


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/rtp/pull/205?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
Base: **87.60**% // Head: **87.83**% // Increases project coverage by **`+0.23%`** :tada:
> Coverage data is based on head [(`82029af`)](https://codecov.io/gh/pion/rtp/pull/205?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) compared to base [(`d13e8b6`)](https://codecov.io/gh/pion/rtp/commit/d13e8b61b96c5985359fd6cd6d88a3115b22cedd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> Patch coverage: 100.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #205      +/-   ##
==========================================
+ Coverage   87.60%   87.83%   +0.23%     
==========================================
  Files          20       20              
  Lines        1831     1842      +11     
==========================================
+ Hits         1604     1618      +14     
+ Misses        197      195       -2     
+ Partials       30       29       -1     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `87.83% <100.00%> (+0.23%)` | :arrow_up: |
| wasm | `86.86% <100.00%> (+0.24%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/rtp/pull/205?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [codecs/vp9\_packet.go](https://codecov.io/gh/pion/rtp/pull/205?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y29kZWNzL3ZwOV9wYWNrZXQuZ28=) | `95.68% <100.00%> (+1.57%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/pion/rtp/pull/205?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).

tmatth: @aler9 it looks like you [already pushed this to the v1 branch](https://github.com/pion/rtp/commit/c3ea6851e25b325c13e082006ea10b40e11f9a75), was that intentional?
aler9: @tmatth i didn't notice it but at the same time the v1 branch must be protected by an administrator in order to avoid conflicts. Anyway this PR is urgent as it allows anyone to crash any pion/webrtc-based software.
tmatth: @aler9 can you rebase?

Patch:
diff --git a/codecs/vp9_packet.go b/codecs/vp9_packet.go
index 917e630..533a3b1 100644
--- a/codecs/vp9_packet.go
+++ b/codecs/vp9_packet.go
@@ -207,12 +207,15 @@ func (p *VP9Packet) Unmarshal(packet []byte) ([]byte, error) {
 
 // Picture ID:
 //
-//      +-+-+-+-+-+-+-+-+
+//	+-+-+-+-+-+-+-+-+
+//
 // I:   |M| PICTURE ID  |   M:0 => picture id is 7 bits.
-//      +-+-+-+-+-+-+-+-+   M:1 => picture id is 15 bits.
+//
+//	+-+-+-+-+-+-+-+-+   M:1 => picture id is 15 bits.
+//
 // M:   | EXTENDED PID  |
-//      +-+-+-+-+-+-+-+-+
 //
+//	+-+-+-+-+-+-+-+-+
 func (p *VP9Packet) parsePictureID(packet []byte, pos int) (int, error) {
 	if len(packet) <= pos {
 		return pos, errShortPacket
@@ -245,10 +248,11 @@ func (p *VP9Packet) parseLayerInfo(packet []byte, pos int) (int, error) {
 
 // Layer indices (flexible mode):
 //
-//      +-+-+-+-+-+-+-+-+
+//	+-+-+-+-+-+-+-+-+
+//
 // L:   |  T  |U|  S  |D|
-//      +-+-+-+-+-+-+-+-+
 //
+//	+-+-+-+-+-+-+-+-+
 func (p *VP9Packet) parseLayerInfoCommon(packet []byte, pos int) (int, error) {
 	if len(packet) <= pos {
 		return pos, errShortPacket
@@ -269,12 +273,13 @@ func (p *VP9Packet) parseLayerInfoCommon(packet []byte, pos int) (int, error) {
 
 // Layer indices (non-flexible mode):
 //
-//      +-+-+-+-+-+-+-+-+
+//	+-+-+-+-+-+-+-+-+
+//
 // L:   |  T  |U|  S  |D|
-//      +-+-+-+-+-+-+-+-+
-//      |   TL0PICIDX   |
-//      +-+-+-+-+-+-+-+-+
 //
+//	+-+-+-+-+-+-+-+-+
+//	|   TL0PICIDX   |
+//	+-+-+-+-+-+-+-+-+
 func (p *VP9Packet) parseLayerInfoNonFlexibleMode(packet []byte, pos int) (int, error) {
 	if len(packet) <= pos {
 		return pos, errShortPacket
@@ -287,11 +292,12 @@ func (p *VP9Packet) parseLayerInfoNonFlexibleMode(packet []byte, pos int) (int,
 
 // Reference indices:
 //
-//      +-+-+-+-+-+-+-+-+                P=1,F=1: At least one reference index
+//	+-+-+-+-+-+-+-+-+                P=1,F=1: At least one reference index
+//
 // P,F: | P_DIFF      |N|  up to 3 times          has to be specified.
-//      +-+-+-+-+-+-+-+-+                    N=1: An additional P_DIFF follows
-//                                                current P_DIFF.
 //
+//	+-+-+-+-+-+-+-+-+                    N=1: An additional P_DIFF follows
+//	                                          current P_DIFF.
 func (p *VP9Packet) parseRefIndices(packet []byte, pos int) (int, error) {
 	for {
 		if len(packet) <= pos {
@@ -313,24 +319,30 @@ func (p *VP9Packet) parseRefIndices(packet []byte, pos int) (int, error) {
 
 // Scalability structure (SS):
 //
-//      +-+-+-+-+-+-+-+-+
+//	+-+-+-+-+-+-+-+-+
+//
 // V:   | N_S |Y|G|-|-|-|
-//      +-+-+-+-+-+-+-+-+              -|
+//
+//	+-+-+-+-+-+-+-+-+              -|
+//
 // Y:   |     WIDTH     | (OPTIONAL)    .
-//      +               +               .
-//      |               | (OPTIONAL)    .
-//      +-+-+-+-+-+-+-+-+               . N_S + 1 times
-//      |     HEIGHT    | (OPTIONAL)    .
-//      +               +               .
-//      |               | (OPTIONAL)    .
-//      +-+-+-+-+-+-+-+-+              -|
+//   - +               .
+//     |               | (OPTIONAL)    .
+//     +-+-+-+-+-+-+-+-+               . N_S + 1 times
+//     |     HEIGHT    | (OPTIONAL)    .
+//   - +               .
+//     |               | (OPTIONAL)    .
+//     +-+-+-+-+-+-+-+-+              -|
+//
 // G:   |      N_G      | (OPTIONAL)
-//      +-+-+-+-+-+-+-+-+                           -|
+//
+//	+-+-+-+-+-+-+-+-+                           -|
+//
 // N_G: |  T  |U| R |-|-| (OPTIONAL)                 .
-//      +-+-+-+-+-+-+-+-+              -|            . N_G times
-//      |    P_DIFF     | (OPTIONAL)    . R times    .
-//      +-+-+-+-+-+-+-+-+              -|           -|
 //
+//	+-+-+-+-+-+-+-+-+              -|            . N_G times
+//	|    P_DIFF     | (OPTIONAL)    . R times    .
+//	+-+-+-+-+-+-+-+-+              -|           -|
 func (p *VP9Packet) parseSSData(packet []byte, pos int) (int, error) {
 	if len(packet) <= pos {
 		return pos, errShortPacket
@@ -348,6 +360,10 @@ func (p *VP9Packet) parseSSData(packet []byte, pos int) (int, error) {
 		p.Width = make([]uint16, NS)
 		p.Height = make([]uint16, NS)
 		for i := 0; i < int(NS); i++ {
+			if len(packet) <= (pos + 3) {
+				return pos, errShortPacket
+			}
+
 			p.Width[i] = uint16(packet[pos])<<8 | uint16(packet[pos+1])
 			pos += 2
 			p.Height[i] = uint16(packet[pos])<<8 | uint16(packet[pos+1])
@@ -356,17 +372,30 @@ func (p *VP9Packet) parseSSData(packet []byte, pos int) (int, error) {
 	}
 
 	if p.G {
+		if len(packet) <= pos {
+			return pos, errShortPacket
+		}
+
 		p.NG = packet[pos]
 		pos++
 	}
 
 	for i := 0; i < int(p.NG); i++ {
+		if len(packet) <= pos {
+			return pos, errShortPacket
+		}
+
 		p.PGTID = append(p.PGTID, packet[pos]>>5)
 		p.PGU = append(p.PGU, packet[pos]&0x10 != 0)
 		R := (packet[pos] >> 2) & 0x3
 		pos++
 
 		p.PGPDiff = append(p.PGPDiff, []uint8{})
+
+		if len(packet) <= (pos + int(R) - 1) {
+			return pos, errShortPacket
+		}
+
 		for j := 0; j < int(R); j++ {
 			p.PGPDiff[i] = append(p.PGPDiff[i], packet[pos])
 			pos++


Test patch:
diff --git a/codecs/vp9_packet_test.go b/codecs/vp9_packet_test.go
index 65861c6..f2c5e2d 100644
--- a/codecs/vp9_packet_test.go
+++ b/codecs/vp9_packet_test.go
@@ -167,6 +167,22 @@ func TestVP9Packet_Unmarshal(t *testing.T) {
 				Payload: []byte{},
 			},
 		},
+		"ScalabilityMissingWidth": {
+			b:   []byte("200"),
+			err: errShortPacket,
+		},
+		"ScalabilityMissingNG": {
+			b:   []byte("b00200000000"),
+			err: errShortPacket,
+		},
+		"ScalabilityMissingTemporalLayerIDs": {
+			b:   []byte("20B0"),
+			err: errShortPacket,
+		},
+		"ScalabilityMissingReferenceIndices": {
+			b:   []byte("20B007"),
+			err: errShortPacket,
+		},
 	}
 	for name, c := range cases {
 		c := c