[Backport] Fix for CVE-2019-5818

Cleanup media BitReader ReadBits() calls

Initialize temporary values, check return values.
Small tweaks to solution proposed by adtolbar@microsoft.com.

Bug: 929962
Change-Id: I06501312c651ef305fe3cfa17f58d5cbac3f95e6
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

3 files changed, 18 insertions, 6 deletions

diff --git a/chromium/media/base/bit_reader_core.cc b/chromium/media/base/bit_reader_core.cc
index 237470c7669..e89971cca51 100644
--- a/chromium/media/base/bit_reader_core.cc
+++ b/chromium/media/base/bit_reader_core.cc
@@ -116,6 +116,7 @@ bool BitReaderCore::ReadBitsInternal(int num_bits, uint64_t* out) {
     // empty the current bit register for that purpose.
     nbits_ = 0;
     reg_ = 0;
+    *out = 0;
     return false;
   }
 
diff --git a/chromium/media/base/bit_reader_core.h b/chromium/media/base/bit_reader_core.h
index 11962987672..1ba856a6986 100644
--- a/chromium/media/base/bit_reader_core.h
+++ b/chromium/media/base/bit_reader_core.h
@@ -56,9 +56,10 @@ class MEDIA_EXPORT BitReaderCore {
   // integer type.
   template<typename T> bool ReadBits(int num_bits, T* out) {
     DCHECK_LE(num_bits, static_cast<int>(sizeof(T) * 8));
-    uint64_t temp;
+    uint64_t temp = 0;
     bool ret = ReadBitsInternal(num_bits, &temp);
-    *out = static_cast<T>(temp);
+    if (ret)
+      *out = static_cast<T>(temp);
     return ret;
   }
 
diff --git a/chromium/media/base/container_names.cc b/chromium/media/base/container_names.cc
index 24d8ea953ff..b644674425e 100644
--- a/chromium/media/base/container_names.cc
+++ b/chromium/media/base/container_names.cc
@@ -73,11 +73,17 @@ static bool StartsWith(const uint8_t* buffer,
 }
 
 // Helper function to read up to 64 bits from a bit stream.
+// TODO(chcunningham): Delete this helper and replace with direct calls to
+// reader that handle read failure. As-is, we hide failure because returning 0
+// is valid for both a successful and failed read.
 static uint64_t ReadBits(BitReader* reader, int num_bits) {
   DCHECK_GE(reader->bits_available(), num_bits);
   DCHECK((num_bits > 0) && (num_bits <= 64));
-  uint64_t value;
-  reader->ReadBits(num_bits, &value);
+  uint64_t value = 0;
+
+  if (!reader->ReadBits(num_bits, &value))
+    return 0;
+
   return value;
 }
 
@@ -304,7 +310,9 @@ static bool CheckDts(const uint8_t* buffer, int buffer_size) {
     reader.SkipBits(6);
 
     // Verify core audio sampling frequency is an allowed value.
-    RCHECK(kSamplingFrequencyValid[ReadBits(&reader, 4)]);
+    size_t sampling_freq_index = ReadBits(&reader, 4);
+    RCHECK(sampling_freq_index < arraysize(kSamplingFrequencyValid));
+    RCHECK(kSamplingFrequencyValid[sampling_freq_index]);
 
     // Verify transmission bit rate is valid.
     RCHECK(ReadBits(&reader, 5) <= 25);
@@ -316,7 +324,9 @@ static bool CheckDts(const uint8_t* buffer, int buffer_size) {
     reader.SkipBits(1 + 1 + 1 + 1);
 
     // Verify extension audio descriptor flag is an allowed value.
-    RCHECK(kExtAudioIdValid[ReadBits(&reader, 3)]);
+    size_t audio_id_index = ReadBits(&reader, 3);
+    RCHECK(audio_id_index < arraysize(kExtAudioIdValid));
+    RCHECK(kExtAudioIdValid[audio_id_index]);
 
     // Skip extended coding flag and audio sync word insertion flag.
     reader.SkipBits(1 + 1);