Repository: pion/rtp
Pull request #71 (2020-06-06 08:43:07)

Fix panic unmarshalling STAP-A
The H264 Unmarshal method could panic
while unmarshalling a STAP-A unit smaller
than the unit's declared size.

codecov[bot]: # [Codecov](https://codecov.io/gh/pion/rtp/pull/71?src=pr&el=h1) Report
> Merging [#71](https://codecov.io/gh/pion/rtp/pull/71?src=pr&el=desc) into [master](https://codecov.io/gh/pion/rtp/commit/3ea35019945910320f917f2aef8a9fbc4ace4d83&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `66.66%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/rtp/pull/71/graphs/tree.svg?width=650&height=150&src=pr&token=4KrbiBsspj)](https://codecov.io/gh/pion/rtp/pull/71?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #71      +/-   ##
==========================================
+ Coverage   90.67%   90.70%   +0.02%     
==========================================
  Files          13       13              
  Lines         622      624       +2     
==========================================
+ Hits          564      566       +2     
  Misses         41       41              
  Partials       17       17              
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `90.70% <66.66%> (+0.02%)` | :arrow_up: |
| #wasm | `90.70% <66.66%> (+0.02%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pion/rtp/pull/71?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [codecs/h264\_packet.go](https://codecov.io/gh/pion/rtp/pull/71/diff?src=pr&el=tree#diff-Y29kZWNzL2gyNjRfcGFja2V0Lmdv) | `95.74% <66.66%> (+0.09%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/rtp/pull/71?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/rtp/pull/71?src=pr&el=footer). Last update [3ea3501...b872fa0](https://codecov.io/gh/pion/rtp/pull/71?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index ff6b84e..efa1d4e 100644
--- a/README.md
+++ b/README.md
@@ -43,6 +43,7 @@ Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contribu
 * [Bao Nguyen](https://github.com/sysbot) *add VP9 noop, bug fixes.
 * [Tarrence van As](https://github.com/tarrencev) *add audio level extension*
 * [Simone Gotti](https://github.com/sgotti)
+* [Guilherme Souza](https://github.com/gqgs)
 
 ### License
 MIT License - see [LICENSE](LICENSE) for full text
diff --git a/codecs/h264_packet.go b/codecs/h264_packet.go
index 4f9890e..b03faa8 100644
--- a/codecs/h264_packet.go
+++ b/codecs/h264_packet.go
@@ -169,8 +169,8 @@ func (p *H264Packet) Unmarshal(payload []byte) ([]byte, error) {
 			naluSize := int(binary.BigEndian.Uint16(payload[currOffset:]))
 			currOffset += stapaNALULengthSize
 
-			if currOffset+len(payload) < currOffset+naluSize {
-				return nil, fmt.Errorf("STAP-A declared size(%d) is larger then buffer(%d)", naluSize, len(payload)-currOffset)
+			if len(payload) < currOffset+naluSize {
+				return nil, fmt.Errorf("STAP-A declared size(%d) is larger than buffer(%d)", naluSize, len(payload)-currOffset)
 			}
 
 			result = append(result, annexbNALUStartCode()...)


Test patch:
diff --git a/codecs/h264_packet_test.go b/codecs/h264_packet_test.go
index e175e58..b346111 100644
--- a/codecs/h264_packet_test.go
+++ b/codecs/h264_packet_test.go
@@ -92,6 +92,8 @@ func TestH264Packet_Unmarshal(t *testing.T) {
 	singlePayloadMultiNALU := []byte{0x78, 0x00, 0x0f, 0x67, 0x42, 0xc0, 0x1f, 0x1a, 0x32, 0x35, 0x01, 0x40, 0x7a, 0x40, 0x3c, 0x22, 0x11, 0xa8, 0x00, 0x05, 0x68, 0x1a, 0x34, 0xe3, 0xc8}
 	singlePayloadMultiNALUUnmarshaled := []byte{0x00, 0x00, 0x00, 0x01, 0x67, 0x42, 0xc0, 0x1f, 0x1a, 0x32, 0x35, 0x01, 0x40, 0x7a, 0x40, 0x3c, 0x22, 0x11, 0xa8, 0x00, 0x00, 0x00, 0x01, 0x68, 0x1a, 0x34, 0xe3, 0xc8}
 
+	incompleteSinglePayloadMultiNALU := []byte{0x78, 0x00, 0x0f, 0x67, 0x42, 0xc0, 0x1f, 0x1a, 0x32, 0x35, 0x01, 0x40, 0x7a, 0x40, 0x3c, 0x22, 0x11}
+
 	pkt := H264Packet{}
 	if _, err := pkt.Unmarshal(nil); err == nil {
 		t.Fatal("Unmarshal did not fail on nil payload")
@@ -105,6 +107,10 @@ func TestH264Packet_Unmarshal(t *testing.T) {
 		t.Fatal("Unmarshal accepted a packet with a NALU Type we don't handle")
 	}
 
+	if _, err := pkt.Unmarshal(incompleteSinglePayloadMultiNALU); err == nil {
+		t.Fatal("Unmarshal accepted a STAP-A packet with insufficient data")
+	}
+
 	res, err := pkt.Unmarshal(singlePayload)
 	if err != nil {
 		t.Fatal(err)