From 1ce585953c7a5546c2aea1025adce7fd22788f85 Mon Sep 17 00:00:00 2001
From: Danil Chapovalov <danilchap@webrtc.org>
Date: Mon, 20 Sep 2021 14:12:23 +0200
Subject: [PATCH] Fix integer underflow in BitstreamReader::ConsumeBits
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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                      |  10 ++++++----
 rtc_base/bitstream_reader_unittest.cc             |  10 ++++++++++
 .../h264-depacketizer-fuzzer-corpus/h264-1        | Bin 0 -> 127 bytes
 3 files changed, 16 insertions(+), 4 deletions(-)
 create mode 100644 test/fuzzers/corpora/h264-depacketizer-fuzzer-corpus/h264-1

diff --git a/rtc_base/bitstream_reader.cc b/rtc_base/bitstream_reader.cc
index 0bb604970b..d2c622d938 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 @@ 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);
 }
 
diff --git a/rtc_base/bitstream_reader_unittest.cc b/rtc_base/bitstream_reader_unittest.cc
index 45c4eca8d4..997abdf573 100644
--- a/rtc_base/bitstream_reader_unittest.cc
+++ b/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);
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 0000000000000000000000000000000000000000..dcb9c476f0bee1366f2def9fb1b8fba3ae6bf5dd
GIT binary patch
literal 127
zcmZQzVB|1yXJBAp`oX}+!N~EST?a|%|NsBCAg&o$#n1o$|8pSme?VkAAo~BaGcf$X
UshRyhNV6=$upj^bb1*Of09HC66#xJL

literal 0
HcmV?d00001