From cd8b36bb9f783a7b102d1937706b4efe63cbebe2 Mon Sep 17 00:00:00 2001
From: Danil Chapovalov <danilchap@webrtc.org>
Date: Fri, 12 Jul 2024 13:52:06 +0200
Subject: [PATCH] Fix integer underflow in H264 bitstream parser

num_ref are represented using golomb and may be very large.
BitstreamReader is generally resilent to many consenquites fail reads, but not when number of reads comparable to int limit.
This change address the issue in two ways, either one is enough, but both are helpful in their own way:

H264 parser now fails faster when number of references is too large.
BitstreamReader now is resilent to unlimited number of fail reads.

Bug: chromium:352402499
Change-Id: I19646bc3f53cd2970393d00bc143400b1fdf5473
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/357100
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42628}
---
 common_video/h264/h264_bitstream_parser.cc | 6 ++++++
 common_video/h264/h264_common.h            | 3 +++
 common_video/h264/pps_parser.cc            | 4 ++++
 rtc_base/bitstream_reader.cc               | 7 ++++---
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/common_video/h264/h264_bitstream_parser.cc b/common_video/h264/h264_bitstream_parser.cc
index d358ad6eda..9f02a96d12 100644
--- a/common_video/h264/h264_bitstream_parser.cc
+++ b/common_video/h264/h264_bitstream_parser.cc
@@ -120,9 +120,15 @@ H264BitstreamParser::Result H264BitstreamParser::ParseNonParameterSetNalu(
       if (slice_reader.Read<bool>()) {
         // num_ref_idx_l0_active_minus1: ue(v)
         num_ref_idx_l0_active_minus1 = slice_reader.ReadExponentialGolomb();
+        if (num_ref_idx_l0_active_minus1 > H264::kMaxReferenceIndex) {
+          return kInvalidStream;
+        }
         if (slice_type == H264::SliceType::kB) {
           // num_ref_idx_l1_active_minus1: ue(v)
           num_ref_idx_l1_active_minus1 = slice_reader.ReadExponentialGolomb();
+          if (num_ref_idx_l1_active_minus1 > H264::kMaxReferenceIndex) {
+            return kInvalidStream;
+          }
         }
       }
       break;
diff --git a/common_video/h264/h264_common.h b/common_video/h264/h264_common.h
index 1bc9867d3f..a55d6470e8 100644
--- a/common_video/h264/h264_common.h
+++ b/common_video/h264/h264_common.h
@@ -33,6 +33,9 @@ const size_t kNaluShortStartSequenceSize = 3;
 // The size of the NALU type byte (1).
 const size_t kNaluTypeSize = 1;
 
+// Maximum reference index for reference pictures.
+constexpr int kMaxReferenceIndex = 31;
+
 enum NaluType : uint8_t {
   kSlice = 1,
   kIdr = 5,
diff --git a/common_video/h264/pps_parser.cc b/common_video/h264/pps_parser.cc
index e2d3eeecf2..46105d5b9f 100644
--- a/common_video/h264/pps_parser.cc
+++ b/common_video/h264/pps_parser.cc
@@ -134,6 +134,10 @@ absl::optional<PpsParser::PpsState> PpsParser::ParseInternal(
   pps.num_ref_idx_l0_default_active_minus1 = reader.ReadExponentialGolomb();
   // num_ref_idx_l1_default_active_minus1: ue(v)
   pps.num_ref_idx_l1_default_active_minus1 = reader.ReadExponentialGolomb();
+  if (pps.num_ref_idx_l0_default_active_minus1 > H264::kMaxReferenceIndex ||
+      pps.num_ref_idx_l1_default_active_minus1 > H264::kMaxReferenceIndex) {
+    return absl::nullopt;
+  }
   // weighted_pred_flag: u(1)
   pps.weighted_pred_flag = reader.Read<bool>();
   // weighted_bipred_idc: u(2)
diff --git a/rtc_base/bitstream_reader.cc b/rtc_base/bitstream_reader.cc
index 3e1b94d8d4..bb23540b37 100644
--- a/rtc_base/bitstream_reader.cc
+++ b/rtc_base/bitstream_reader.cc
@@ -26,7 +26,7 @@ uint64_t BitstreamReader::ReadBits(int bits) {
   set_last_read_is_verified(false);
 
   if (remaining_bits_ < bits) {
-    remaining_bits_ -= bits;
+    Invalidate();
     return 0;
   }
 
@@ -64,10 +64,11 @@ uint64_t BitstreamReader::ReadBits(int bits) {
 
 int BitstreamReader::ReadBit() {
   set_last_read_is_verified(false);
-  --remaining_bits_;
-  if (remaining_bits_ < 0) {
+  if (remaining_bits_ <= 0) {
+    Invalidate();
     return 0;
   }
+  --remaining_bits_;
 
   int bit_position = remaining_bits_ % 8;
   if (bit_position == 0) {