Fail RTP parsing on excessive padding length.
BUG=webrtc:4771
R=stefan@webrtc.org
Review URL: https://codereview.webrtc.org/1220863002
Cr-Commit-Position: refs/heads/master@{#9525}
diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc
index cc5d65c..e67ec4e 100644
--- a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc
@@ -402,7 +402,7 @@
}
TEST_F(ReceiverFecTest, TruncatedPacketWithFBitSetEndingAfterFirstRedHeader) {
- const uint8_t kPacket[] = {0xa9,
+ const uint8_t kPacket[] = {0x89,
0x27,
0x3a,
0x83,
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc
index df3067a..8e2ff17 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc
@@ -237,7 +237,8 @@
size_t* packet_length,
uint32_t original_ssrc,
const RTPHeader& header) const {
- if (kRtxHeaderSize + header.headerLength > *packet_length) {
+ if (kRtxHeaderSize + header.headerLength + header.paddingLength >
+ *packet_length) {
return false;
}
const uint8_t* rtx_header = packet + header.headerLength;
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc
index 8d8147c..ff64e49 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc
@@ -13,6 +13,7 @@
#include <assert.h>
#include <string.h>
+#include "webrtc/base/checks.h"
#include "webrtc/modules/rtp_rtcp/interface/rtp_cvo.h"
#include "webrtc/modules/rtp_rtcp/interface/rtp_payload_registry.h"
#include "webrtc/modules/rtp_rtcp/source/rtp_format.h"
@@ -60,6 +61,7 @@
rtp_header->header.timestamp);
rtp_header->type.Video.codec = specific_payload.Video.videoCodecType;
+ DCHECK_GE(payload_length, rtp_header->header.paddingLength);
const size_t payload_data_length =
payload_length - rtp_header->header.paddingLength;
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 566772a..23300bb 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -770,8 +770,10 @@
EXPECT_EQ(kMaxPaddingLength + rtp_header_len,
transport_.last_sent_packet_len_);
// Parse sent packet.
- ASSERT_TRUE(rtp_parser->Parse(transport_.last_sent_packet_, kPaddingBytes,
+ ASSERT_TRUE(rtp_parser->Parse(transport_.last_sent_packet_,
+ transport_.last_sent_packet_len_,
&rtp_header));
+ EXPECT_EQ(kMaxPaddingLength, rtp_header.paddingLength);
// Verify sequence number and timestamp.
EXPECT_EQ(seq_num++, rtp_header.sequenceNumber);
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_utility.cc b/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
index 3fef053..0d083bd 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
@@ -355,6 +355,8 @@
}
header.headerLength += XLen;
}
+ if (header.headerLength + header.paddingLength > static_cast<size_t>(length))
+ return false;
return true;
}
diff --git a/webrtc/video/rampup_tests.cc b/webrtc/video/rampup_tests.cc
index e4da1975..44cbb70 100644
--- a/webrtc/video/rampup_tests.cc
+++ b/webrtc/video/rampup_tests.cc
@@ -131,7 +131,9 @@
++total_packets_sent_;
if (header.paddingLength > 0)
++padding_packets_sent_;
- if (rtx_media_ssrcs_.find(header.ssrc) != rtx_media_ssrcs_.end()) {
+ // Handle RTX retransmission, but only for non-padding-only packets.
+ if (rtx_media_ssrcs_.find(header.ssrc) != rtx_media_ssrcs_.end() &&
+ header.headerLength + header.paddingLength != length) {
rtx_media_sent_ += length - header.headerLength - header.paddingLength;
if (header.paddingLength == 0)
++rtx_media_packets_sent_;
@@ -141,9 +143,8 @@
EXPECT_TRUE(payload_registry_->RestoreOriginalPacket(
&restored_packet_ptr, packet, &restored_length,
rtx_media_ssrcs_[header.ssrc], header));
- length = restored_length;
- EXPECT_TRUE(rtp_parser_->Parse(
- restored_packet, static_cast<int>(length), &header));
+ EXPECT_TRUE(
+ rtp_parser_->Parse(restored_packet_ptr, restored_length, &header));
} else {
rtp_rtcp_->SetRemoteSSRC(header.ssrc);
}
