diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc index e67ec4e53..f64b537a5 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc @@ -18,6 +18,7 @@ #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/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/fec_test_helper.h" #include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h" @@ -82,6 +83,7 @@ class ReceiverFecTest : public ::testing::Test { delete red_packet; } + void InjectGarbagePacketLength(size_t fec_garbage_offset); static void SurvivesMaliciousPacket(const uint8_t* data, size_t length, uint8_t ulpfec_payload_type); @@ -109,8 +111,7 @@ TEST_F(ReceiverFecTest, TwoMediaOneFec) { // Recovery std::list::iterator it = media_rtp_packets.begin(); - std::list::iterator media_it = media_rtp_packets.begin(); - BuildAndAddRedMediaPacket(*media_it); + BuildAndAddRedMediaPacket(*it); VerifyReconstructedMediaPacket(*it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); // Drop one media packet. @@ -128,6 +129,44 @@ TEST_F(ReceiverFecTest, TwoMediaOneFec) { DeletePackets(&media_packets); } +void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { + EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) + .WillRepeatedly(Return(true)); + + const unsigned int kNumFecPackets = 1u; + std::list media_rtp_packets; + std::list media_packets; + GenerateFrame(2, 0, &media_rtp_packets, &media_packets); + std::list fec_packets; + GenerateFEC(&media_packets, &fec_packets, kNumFecPackets); + ByteWriter::WriteBigEndian( + &fec_packets.front()->data[fec_garbage_offset], 0x4711); + + // Inject first media packet, then first FEC packet, skipping the second media + // packet to cause a recovery from the FEC packet. + BuildAndAddRedMediaPacket(media_rtp_packets.front()); + BuildAndAddRedFecPacket(fec_packets.front()); + EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + + FecPacketCounter counter = receiver_fec_->GetPacketCounter(); + EXPECT_EQ(2u, counter.num_packets); + EXPECT_EQ(1u, counter.num_fec_packets); + EXPECT_EQ(0u, counter.num_recovered_packets); + + DeletePackets(&media_packets); +} + +TEST_F(ReceiverFecTest, InjectGarbageFecHeaderLengthRecovery) { + // Byte offset 8 is the 'length recovery' field of the FEC header. + InjectGarbagePacketLength(8); +} + +TEST_F(ReceiverFecTest, InjectGarbageFecLevelHeaderProtectionLength) { + // Byte offset 10 is the 'protection length' field in the first FEC level + // header. + InjectGarbagePacketLength(10); +} + TEST_F(ReceiverFecTest, TwoMediaTwoFec) { const unsigned int kNumFecPackets = 2u; std::list media_rtp_packets; diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc b/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc index a9e88a5bc..48126cec5 100644 --- a/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc +++ b/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc @@ -634,23 +634,35 @@ void ForwardErrorCorrection::InsertPackets( DiscardOldPackets(recovered_packet_list); } -void ForwardErrorCorrection::InitRecovery(const FecPacket* fec_packet, +bool ForwardErrorCorrection::InitRecovery(const FecPacket* fec_packet, RecoveredPacket* recovered) { // This is the first packet which we try to recover with. const uint16_t ulp_header_size = fec_packet->pkt->data[0] & 0x40 ? kUlpHeaderSizeLBitSet : kUlpHeaderSizeLBitClear; // L bit set? + if (fec_packet->pkt->length < + static_cast(kFecHeaderSize + ulp_header_size)) { + LOG(LS_WARNING) + << "Truncated FEC packet doesn't contain room for ULP header."; + return false; + } recovered->pkt = new Packet; memset(recovered->pkt->data, 0, IP_PACKET_SIZE); recovered->returned = false; recovered->was_recovered = true; - uint8_t protection_length[2]; - // Copy the protection length from the ULP header. - memcpy(protection_length, &fec_packet->pkt->data[10], 2); + uint16_t protection_length = + ByteReader::ReadBigEndian(&fec_packet->pkt->data[10]); + if (protection_length > + std::min( + sizeof(recovered->pkt->data) - kRtpHeaderSize, + sizeof(fec_packet->pkt->data) - kFecHeaderSize - ulp_header_size)) { + LOG(LS_WARNING) << "Incorrect FEC protection length, dropping."; + return false; + } // Copy FEC payload, skipping the ULP header. memcpy(&recovered->pkt->data[kRtpHeaderSize], &fec_packet->pkt->data[kFecHeaderSize + ulp_header_size], - ByteReader::ReadBigEndian(protection_length)); + protection_length); // Copy the length recovery field. memcpy(recovered->length_recovery, &fec_packet->pkt->data[8], 2); // Copy the first 2 bytes of the FEC header. @@ -660,9 +672,10 @@ void ForwardErrorCorrection::InitRecovery(const FecPacket* fec_packet, // Set the SSRC field. ByteWriter::WriteBigEndian(&recovered->pkt->data[8], fec_packet->ssrc); + return true; } -void ForwardErrorCorrection::FinishRecovery(RecoveredPacket* recovered) { +bool ForwardErrorCorrection::FinishRecovery(RecoveredPacket* recovered) { // Set the RTP version to 2. recovered->pkt->data[0] |= 0x80; // Set the 1st bit. recovered->pkt->data[0] &= 0xbf; // Clear the 2nd bit. @@ -674,6 +687,10 @@ void ForwardErrorCorrection::FinishRecovery(RecoveredPacket* recovered) { recovered->pkt->length = ByteReader::ReadBigEndian(recovered->length_recovery) + kRtpHeaderSize; + if (recovered->pkt->length > sizeof(recovered->pkt->data) - kRtpHeaderSize) + return false; + + return true; } void ForwardErrorCorrection::XorPackets(const Packet* src_packet, @@ -700,9 +717,11 @@ void ForwardErrorCorrection::XorPackets(const Packet* src_packet, } } -void ForwardErrorCorrection::RecoverPacket( - const FecPacket* fec_packet, RecoveredPacket* rec_packet_to_insert) { - InitRecovery(fec_packet, rec_packet_to_insert); +bool ForwardErrorCorrection::RecoverPacket( + const FecPacket* fec_packet, + RecoveredPacket* rec_packet_to_insert) { + if (!InitRecovery(fec_packet, rec_packet_to_insert)) + return false; ProtectedPacketList::const_iterator protected_it = fec_packet->protected_pkt_list.begin(); while (protected_it != fec_packet->protected_pkt_list.end()) { @@ -714,7 +733,9 @@ void ForwardErrorCorrection::RecoverPacket( } ++protected_it; } - FinishRecovery(rec_packet_to_insert); + if (!FinishRecovery(rec_packet_to_insert)) + return false; + return true; } void ForwardErrorCorrection::AttemptRecover( @@ -729,7 +750,13 @@ void ForwardErrorCorrection::AttemptRecover( // Recovery possible. RecoveredPacket* packet_to_insert = new RecoveredPacket; packet_to_insert->pkt = NULL; - RecoverPacket(*fec_packet_list_it, packet_to_insert); + if (!RecoverPacket(*fec_packet_list_it, packet_to_insert)) { + // Can't recover using this packet, drop it. + DiscardFECPacket(*fec_packet_list_it); + fec_packet_list_it = fec_packet_list_.erase(fec_packet_list_it); + delete packet_to_insert; + continue; + } // Add recovered packet to the list of recovered packets and update any // FEC packets covering this packet with a pointer to the data. diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction.h b/webrtc/modules/rtp_rtcp/source/forward_error_correction.h index c6f273848..dbb3fa93e 100644 --- a/webrtc/modules/rtp_rtcp/source/forward_error_correction.h +++ b/webrtc/modules/rtp_rtcp/source/forward_error_correction.h @@ -279,7 +279,7 @@ class ForwardErrorCorrection { void AttemptRecover(RecoveredPacketList* recovered_packet_list); // Initializes the packet recovery using the FEC packet. - static void InitRecovery(const FecPacket* fec_packet, + static bool InitRecovery(const FecPacket* fec_packet, RecoveredPacket* recovered); // Performs XOR between |src_packet| and |dst_packet| and stores the result @@ -287,10 +287,10 @@ class ForwardErrorCorrection { static void XorPackets(const Packet* src_packet, RecoveredPacket* dst_packet); // Finish up the recovery of a packet. - static void FinishRecovery(RecoveredPacket* recovered); + static bool FinishRecovery(RecoveredPacket* recovered); // Recover a missing packet. - void RecoverPacket(const FecPacket* fec_packet, + bool RecoverPacket(const FecPacket* fec_packet, RecoveredPacket* rec_packet_to_insert); // Get the number of missing media packets which are covered by this