Prevent OOB reads for truncated H264 STAP-A packets.

BUG=webrtc:4771, webrtc:4834
R=stefan@webrtc.org

Review URL: https://codereview.webrtc.org/1238033003

Cr-Commit-Position: refs/heads/master@{#9650}

webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc

@@ -40,6 +40,23 @@ enum NalDefs { kFBit = 0x80, kNriMask = 0x60, kTypeMask = 0x1F };
 // Bit masks for FU (A and B) headers.
 enum FuDefs { kSBit = 0x80, kEBit = 0x40, kRBit = 0x20 };
 
+// TODO(pbos): Avoid parsing this here as well as inside the jitter buffer.
+bool VerifyStapANaluLengths(const uint8_t* nalu_ptr, size_t length_remaining) {
+  while (length_remaining > 0) {
+    // Buffer doesn't contain room for additional nalu length.
+    if (length_remaining < sizeof(uint16_t))
+      return false;
+    uint16_t nalu_size = nalu_ptr[0] << 8 | nalu_ptr[1];
+    nalu_ptr += sizeof(uint16_t);
+    length_remaining -= sizeof(uint16_t);
+    if (nalu_size > length_remaining)
+      return false;
+    nalu_ptr += nalu_size;
+    length_remaining -= nalu_size;
+  }
+  return true;
+}
+
 bool ParseSingleNalu(RtpDepacketizer::ParsedPayload* parsed_payload,
                      const uint8_t* payload_data,
                      size_t payload_data_length) {
@@ -59,6 +76,11 @@ bool ParseSingleNalu(RtpDepacketizer::ParsedPayload* parsed_payload,
       LOG(LS_ERROR) << "StapA header truncated.";
       return false;
     }
+    if (!VerifyStapANaluLengths(nalu_start, nalu_length)) {
+      LOG(LS_ERROR) << "StapA packet with incorrect NALU packet lengths.";
+      return false;
+    }
+
     nal_type = payload_data[kStapAHeaderSize] & kTypeMask;
     nalu_start += kStapAHeaderSize;
     nalu_length -= kStapAHeaderSize;

webrtc/modules/video_coding/main/source/session_info.cc

@@ -133,6 +133,8 @@ size_t VCMSessionInfo::InsertBuffer(uint8_t* frame_buffer,
 
   // We handle H.264 STAP-A packets in a special way as we need to remove the
   // two length bytes between each NAL unit, and potentially add start codes.
+  // TODO(pbos): Remove H264 parsing from this step and use a fragmentation
+  // header supplied by the H264 depacketizer.
   const size_t kH264NALHeaderLengthInBytes = 1;
   const size_t kLengthFieldLength = 2;
   if (packet.codecSpecificHeader.codec == kRtpVideoH264 &&

webrtc/test/call_test.cc

@@ -132,6 +132,7 @@ void CallTest::CreateFrameGeneratorCapturer() {
                                            stream.max_framerate,
                                            clock_));
 }
+
 void CallTest::CreateStreams() {
   assert(send_stream_ == NULL);
   assert(receive_streams_.empty());

webrtc/video/packet_injection_tests.cc

@@ -0,0 +1,91 @@
+/*
+ *  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 "testing/gtest/include/gtest/gtest.h"
+
+#include "webrtc/test/call_test.h"
+#include "webrtc/test/null_transport.h"
+
+namespace webrtc {
+
+class PacketInjectionTest : public test::CallTest {
+ protected:
+  enum class CodecType {
+    kVp8,
+    kH264,
+  };
+
+  PacketInjectionTest() : rtp_header_parser_(RtpHeaderParser::Create()) {}
+
+  void InjectIncorrectPacket(CodecType codec_type,
+                             uint8_t packet_type,
+                             const uint8_t* packet,
+                             size_t length);
+
+  rtc::scoped_ptr<RtpHeaderParser> rtp_header_parser_;
+};
+
+void PacketInjectionTest::InjectIncorrectPacket(CodecType codec_type,
+                                                uint8_t payload_type,
+                                                const uint8_t* packet,
+                                                size_t length) {
+  test::NullTransport transport;
+  CreateSenderCall(Call::Config(&transport));
+  CreateReceiverCall(Call::Config(&transport));
+
+  CreateSendConfig(1);
+  CreateMatchingReceiveConfigs();
+  receive_configs_[0].decoders[0].payload_type = payload_type;
+  switch (codec_type) {
+    case CodecType::kVp8:
+      receive_configs_[0].decoders[0].payload_name = "VP8";
+      break;
+    case CodecType::kH264:
+      receive_configs_[0].decoders[0].payload_name = "H264";
+      break;
+  }
+  CreateStreams();
+
+  RTPHeader header;
+  EXPECT_TRUE(rtp_header_parser_->Parse(packet, length, &header));
+  EXPECT_EQ(kSendSsrcs[0], header.ssrc)
+      << "Packet should have configured SSRC to not be dropped early.";
+  EXPECT_EQ(payload_type, header.payloadType);
+  Start();
+  EXPECT_EQ(PacketReceiver::DELIVERY_PACKET_ERROR,
+            receiver_call_->Receiver()->DeliverPacket(MediaType::VIDEO, packet,
+                                                      length));
+  Stop();
+
+  DestroyStreams();
+}
+
+TEST_F(PacketInjectionTest, StapAPacketWithTruncatedNalUnits) {
+  const uint8_t kPacket[] = {0x80,
+                             0xE5,
+                             0xE6,
+                             0x0,
+                             0x0,
+                             0xED,
+                             0x23,
+                             0x4,
+                             0x00,
+                             0xC0,
+                             0xFF,
+                             0xED,
+                             0x58,
+                             0xCB,
+                             0xED,
+                             0xDF};
+
+  InjectIncorrectPacket(CodecType::kH264, 101, kPacket, sizeof(kPacket));
+}
+
+}  // namespace webrtc

webrtc/webrtc_tests.gypi

@@ -149,6 +149,7 @@
         'tools/agc/agc_manager_unittest.cc',
         'video/bitrate_estimator_tests.cc',
         'video/end_to_end_tests.cc',
+        'video/packet_injection_tests.cc',
         'video/send_statistics_proxy_unittest.cc',
         'video/video_capture_input_unittest.cc',
         'video/video_decoder_unittest.cc',