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}
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc b/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc
index ba41c62..8849732 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc
@@ -40,6 +40,23 @@
// 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 @@
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;
diff --git a/webrtc/modules/video_coding/main/source/session_info.cc b/webrtc/modules/video_coding/main/source/session_info.cc
index 8eba432..49839e5 100644
--- a/webrtc/modules/video_coding/main/source/session_info.cc
+++ b/webrtc/modules/video_coding/main/source/session_info.cc
@@ -133,6 +133,8 @@
// 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 &&
diff --git a/webrtc/test/call_test.cc b/webrtc/test/call_test.cc
index 2548659..185ed26 100644
--- a/webrtc/test/call_test.cc
+++ b/webrtc/test/call_test.cc
@@ -132,6 +132,7 @@
stream.max_framerate,
clock_));
}
+
void CallTest::CreateStreams() {
assert(send_stream_ == NULL);
assert(receive_streams_.empty());
diff --git a/webrtc/video/packet_injection_tests.cc b/webrtc/video/packet_injection_tests.cc
new file mode 100644
index 0000000..7e1d98f
--- /dev/null
+++ b/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
diff --git a/webrtc/webrtc_tests.gypi b/webrtc/webrtc_tests.gypi
index 2de51ce..9d302f1 100644
--- a/webrtc/webrtc_tests.gypi
+++ b/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',