Fix integer underflow in BitstreamReader::ConsumeBits

Unlike ReadBits, ConsumeBits doesn't limit number of bits it may advance,
and thus should work when that number is close to the integer limit

Bug: chromium:1250730
Change-Id: Ia7847869ef9d3fc16450d572c9e2be6e1aa36741
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232332
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35042}

rtc_base/bitstream_reader.cc

@@ -12,6 +12,8 @@
 
 #include <stdint.h>
 
+#include <limits>
+
 #include "absl/numeric/bits.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/numerics/safe_conversions.h"
@@ -79,14 +81,14 @@ int BitstreamReader::ReadBit() {
 void BitstreamReader::ConsumeBits(int bits) {
   RTC_DCHECK_GE(bits, 0);
   set_last_read_is_verified(false);
+  if (remaining_bits_ < bits) {
+    Invalidate();
+    return;
+  }
 
   int remaining_bytes = (remaining_bits_ + 7) / 8;
   remaining_bits_ -= bits;
   int new_remaining_bytes = (remaining_bits_ + 7) / 8;
-  // When `remaining_bits_` is negative, `BitstreamReader` is in failure state
-  // and `bytes_' member no longer used, thus its value doesn't matter.
-  // In such case it doesn't matter that negative integer division rounds up
-  // instead of down and thus this byte adjustement might seem incorrect.
   bytes_ += (remaining_bytes - new_remaining_bytes);
 }
 

rtc_base/bitstream_reader_unittest.cc

@@ -75,6 +75,16 @@ TEST(BitstreamReaderTest, ConsumeBits) {
   EXPECT_LT(reader.RemainingBitCount(), 0);
 }
 
+TEST(BitstreamReaderTest, ConsumeLotsOfBits) {
+  const uint8_t bytes[1] = {};
+  BitstreamReader reader(bytes);
+
+  reader.ConsumeBits(std::numeric_limits<int>::max());
+  reader.ConsumeBits(std::numeric_limits<int>::max());
+  EXPECT_GE(reader.ReadBit(), 0);
+  EXPECT_FALSE(reader.Ok());
+}
+
 TEST(BitstreamReaderTest, ReadBit) {
   const uint8_t bytes[] = {0b0100'0001, 0b1011'0001};
   BitstreamReader reader(bytes);