commit 1ce585953c7a5546c2aea1025adce7fd22788f85
author Danil Chapovalov <danilchap@webrtc.org> Mon Sep 20 12:12:23 2021
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Sep 20 19:37:49 2021
tree a875572dc12d314b6242b65d4e8f9ce6715312f3
parent b3b102cdb825213a8d4380012e29e87f2467e659

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

diff --git a/rtc_base/bitstream_reader.cc b/rtc_base/bitstream_reader.cc
index 0bb6049..d2c622d 100644
--- a/rtc_base/bitstream_reader.cc
+++ b/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 @@
 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

diff --git a/rtc_base/bitstream_reader_unittest.cc b/rtc_base/bitstream_reader_unittest.cc
index 45c4eca..997abdf 100644
--- a/rtc_base/bitstream_reader_unittest.cc
+++ b/rtc_base/bitstream_reader_unittest.cc
@@ -75,6 +75,16 @@
   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);

test/fuzzers/corpora/h264-depacketizer-fuzzer-corpus/h264-1

diff --git a/test/fuzzers/corpora/h264-depacketizer-fuzzer-corpus/h264-1 b/test/fuzzers/corpora/h264-depacketizer-fuzzer-corpus/h264-1
new file mode 100644
index 0000000..dcb9c47
--- /dev/null
+++ b/test/fuzzers/corpora/h264-depacketizer-fuzzer-corpus/h264-1
Binary files differ