H264: Fix stap-a-to-annex-b loop over-read
While converting the aggregated (stap-a) packet transform packet
framing input into an annex-b framing copy, the two loops (both the
required size calculation and the stap-a-to-annex-b copy) may
over-read the input buffer.
In both buffers, `nalu_ptr` follows the input (stap-a) buffer, which
is located in `data`, and whose length is `data_size`. Buffer is read
until `nalu_ptr` reaches the end of the buffer. Issues is that the 5th
line in the loop:
```
uint16_t segment_length = nalu_ptr[0] << 8 | nalu_ptr[1];
```
This line accesses `nalu_ptr[1]`, which needs to be protected in
the loop condition. Let's assume `data_size = 4`, and that we restart
the loop with `nalu_ptr = data + 3`. The condition of the loop does
hold (`nalu_ptr = data + 3 < data + data_size`), but the 5th line
will access to `data[3+1] = data[4]`, which is an over-read.
Tested:
```
$ ninja -C out/Default
$ out/Default/modules_unittests --gtest_filter=PacketBuffer*:H264*:RtpPacketizerH264Test*:VideoRtpDepacketizerH264Test*:TestH264SpsPpsTracker* --logs
...
[ PASSED ] 97 tests.
```
Change-Id: I8b8aaf7d12b0bb154430b8922f099cd49e684762
Bug: webrtc:11698
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177140
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Niklas Enbom <niklas.enbom@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31561}
diff --git a/modules/video_coding/h264_sps_pps_tracker.cc b/modules/video_coding/h264_sps_pps_tracker.cc
index 3965b28..4becdb7 100644
--- a/modules/video_coding/h264_sps_pps_tracker.cc
+++ b/modules/video_coding/h264_sps_pps_tracker.cc
@@ -49,6 +49,7 @@
RTPVideoHeader* video_header) {
RTC_DCHECK(video_header);
RTC_DCHECK(video_header->codec == kVideoCodecH264);
+ RTC_DCHECK_GT(bitstream.size(), 0);
auto& h264_header =
absl::get<RTPVideoHeaderH264>(video_header->video_type_header);
@@ -128,7 +129,7 @@
if (h264_header.packetization_type == kH264StapA) {
const uint8_t* nalu_ptr = bitstream.data() + 1;
- while (nalu_ptr < bitstream.data() + bitstream.size()) {
+ while (nalu_ptr < bitstream.data() + bitstream.size() - 1) {
RTC_DCHECK(video_header->is_first_packet_in_frame);
required_size += sizeof(start_code_h264);
@@ -180,7 +181,7 @@
// Copy the rest of the bitstream and insert start codes.
if (h264_header.packetization_type == kH264StapA) {
const uint8_t* nalu_ptr = bitstream.data() + 1;
- while (nalu_ptr < bitstream.data() + bitstream.size()) {
+ while (nalu_ptr < bitstream.data() + bitstream.size() - 1) {
fixed.bitstream.AppendData(start_code_h264);
// The first two bytes describe the length of a segment.
