Fix the issue of undefined-shift in VP8GetBit.

webrtc/modules/video_coding/utility/vp8_header_parser.cc

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 #include "webrtc/modules/video_coding/utility/vp8_header_parser.h"
 
 #include "webrtc/base/logging.h"
 
 namespace webrtc {
 
 namespace vp8 {
 namespace {
 const size_t kCommonPayloadHeaderLength = 3;
 const size_t kKeyPayloadHeaderLength = 10;
+const size_t kParseError = 0;
 }  // namespace
 
 static uint32_t BSwap32(uint32_t x) {
   return (x >> 24) | ((x >> 8) & 0xff00) | ((x << 8) & 0xff0000) | (x << 24);
 }
 
 static void VP8LoadFinalBytes(VP8BitReader* const br) {
   // Only read 8bits at a time.
   if (br->buf_ < br->buf_end_) {
     br->bits_ += 8;
@@ skipping 41 matching lines @@
 
 // Read a bit with proba 'prob'.
 static int VP8GetBit(VP8BitReader* const br, int prob) {
   uint8_t range = br->range_;
   if (br->bits_ < 0) {
     VP8LoadNewBytes(br);
   }
 
   const int pos = br->bits_;
   const uint8_t split = (range * prob) >> 8;
+  if (pos < 0) {
+    pos = 0;

[pbos-webrtc, 2016/04/15 08:58:58]

Also log an error in here, but I think you should do that when bits_ is read
instead and not here. This is not where the error occurs, br->bits_ shouldn't be
able to be negative right?

[jackychen_, 2016/04/15 20:24:50]

Right, it must be an invalid bitstream.

+    kParseError = 1;

[pbos-webrtc, 2016/04/15 08:58:58]

Does this even compile? You shouldn't modify global state in this code.

[jackychen_, 2016/04/15 20:24:50]

Done.

+  }
   const uint8_t value = static_cast<uint8_t>(br->value_ >> pos);
   int bit;
   if (value > split) {
     range -= split + 1;
     br->value_ -= static_cast<uint32_t>(split + 1) << pos;
     bit = 1;
   } else {
     range = split;
     bit = 0;
   }
@@ skipping 72 matching lines @@
   if (length < kCommonPayloadHeaderLength) {
     LOG(LS_WARNING) << "Failed to get QP, invalid length.";
     return false;
   }
   VP8BitReader br;
   const uint32_t bits = buf[0] | (buf[1] << 8) | (buf[2] << 16);
   int key_frame = !(bits & 1);
   // Size of first partition in bytes.
   uint32_t partition_length = (bits >> 5);
   size_t header_length = kCommonPayloadHeaderLength;
+  kParseError = 0;

[pbos-webrtc, 2016/04/15 08:58:58]

Should this not be a property of the bitreader?

[jackychen_, 2016/04/15 20:24:50]

Done.

   if (key_frame) {
     header_length = kKeyPayloadHeaderLength;
   }
   if (header_length + partition_length > length) {
     LOG(LS_WARNING) << "Failed to get QP, invalid length: " << length;
     return false;
   }
   buf += header_length;
 
   VP8InitBitReader(&br, buf, buf + partition_length);
   if (key_frame) {
     // Color space and pixel type.
     VP8Get(&br);
     VP8Get(&br);
   }
   ParseSegmentHeader(&br);
   ParseFilterHeader(&br);
   // Number of coefficient data partitions.
   VP8GetValue(&br, 2);
   // Base QP.
   const int base_q0 = VP8GetValue(&br, 7);
   if (br.eof_ == 1) {
     LOG(LS_WARNING) << "Failed to get QP, end of file reached.";
     return false;
   }
   *qp = base_q0;
+  if (kParseError)
+    return false;
   return true;
 }
 
 }  // namespace vp8
 
 }  // namespace webrtc