diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc index 9d9550b0b..ce240860a 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc @@ -81,11 +81,16 @@ int32_t FecReceiverImpl::AddReceivedRedPacket( uint8_t REDHeaderLength = 1; size_t payload_data_length = packet_length - header.headerLength; + if (payload_data_length == 0) { + LOG(LS_WARNING) << "Corrupt/truncated FEC packet."; + return -1; + } + // Add to list without RED header, aka a virtual RTP packet // we remove the RED header - ForwardErrorCorrection::ReceivedPacket* received_packet = - new ForwardErrorCorrection::ReceivedPacket; + rtc::scoped_ptr received_packet( + new ForwardErrorCorrection::ReceivedPacket); received_packet->pkt = new ForwardErrorCorrection::Packet; // get payload type from RED header @@ -99,16 +104,18 @@ int32_t FecReceiverImpl::AddReceivedRedPacket( if (incoming_rtp_packet[header.headerLength] & 0x80) { // f bit set in RED header REDHeaderLength = 4; + if (payload_data_length < REDHeaderLength) { + LOG(LS_WARNING) << "Corrupt/truncated FEC packet."; + return -1; + } + uint16_t timestamp_offset = (incoming_rtp_packet[header.headerLength + 1]) << 8; timestamp_offset += incoming_rtp_packet[header.headerLength + 2]; timestamp_offset = timestamp_offset >> 2; if (timestamp_offset != 0) { - // |timestampOffset| should be 0. However, it's possible this is the first - // location a corrupt payload can be caught, so don't assert. LOG(LS_WARNING) << "Corrupt payload found."; - delete received_packet; return -1; } @@ -118,21 +125,18 @@ int32_t FecReceiverImpl::AddReceivedRedPacket( // check next RED header if (incoming_rtp_packet[header.headerLength + 4] & 0x80) { - // more than 2 blocks in packet not supported - delete received_packet; - assert(false); + LOG(LS_WARNING) << "More than 2 blocks in packet not supported."; return -1; } if (blockLength > payload_data_length - REDHeaderLength) { - // block length longer than packet - delete received_packet; - assert(false); + LOG(LS_WARNING) << "Block length longer than packet."; return -1; } } ++packet_counter_.num_packets; - ForwardErrorCorrection::ReceivedPacket* second_received_packet = NULL; + rtc::scoped_ptr + second_received_packet; if (blockLength > 0) { // handle block length, split into 2 packets REDHeaderLength = 5; @@ -154,7 +158,7 @@ int32_t FecReceiverImpl::AddReceivedRedPacket( received_packet->pkt->length = blockLength; - second_received_packet = new ForwardErrorCorrection::ReceivedPacket; + second_received_packet.reset(new ForwardErrorCorrection::ReceivedPacket); second_received_packet->pkt = new ForwardErrorCorrection::Packet; second_received_packet->is_fec = true; @@ -202,14 +206,12 @@ int32_t FecReceiverImpl::AddReceivedRedPacket( } if (received_packet->pkt->length == 0) { - delete second_received_packet; - delete received_packet; return 0; } - received_packet_list_.push_back(received_packet); + received_packet_list_.push_back(received_packet.release()); if (second_received_packet) { - received_packet_list_.push_back(second_received_packet); + received_packet_list_.push_back(second_received_packet.release()); } return 0; } diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc index 31baf4e76..3cd2a9e46 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc @@ -16,6 +16,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/modules/rtp_rtcp/interface/fec_receiver.h" +#include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h" #include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/source/fec_test_helper.h" #include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h" @@ -81,6 +82,10 @@ class ReceiverFecTest : public ::testing::Test { delete red_packet; } + static void SurvivesMaliciousPacket(const uint8_t* data, + size_t length, + uint8_t ulpfec_payload_type); + MockRtpData rtp_data_callback_; rtc::scoped_ptr fec_; rtc::scoped_ptr receiver_fec_; @@ -362,4 +367,38 @@ TEST_F(ReceiverFecTest, OldFecPacketDropped) { DeletePackets(&media_packets); } +void ReceiverFecTest::SurvivesMaliciousPacket(const uint8_t* data, + size_t length, + uint8_t ulpfec_payload_type) { + webrtc::RTPHeader header; + rtc::scoped_ptr parser( + webrtc::RtpHeaderParser::Create()); + ASSERT_TRUE(parser->Parse(data, length, &header)); + + webrtc::NullRtpData null_callback; + rtc::scoped_ptr receiver_fec( + webrtc::FecReceiver::Create(&null_callback)); + + receiver_fec->AddReceivedRedPacket(header, data, length, ulpfec_payload_type); +} + +TEST_F(ReceiverFecTest, TruncatedPacketWithFBitSet) { + const uint8_t kTruncatedPacket[] = {0x80, + 0x2a, + 0x68, + 0x71, + 0x29, + 0xa1, + 0x27, + 0x3a, + 0x29, + 0x12, + 0x2a, + 0x98, + 0xe0, + 0x29}; + + SurvivesMaliciousPacket(kTruncatedPacket, sizeof(kTruncatedPacket), 100); +} + } // namespace webrtc