From fdca66910a09254a40735ea39ea68036ebd339c2 Mon Sep 17 00:00:00 2001 From: asapersson Date: Tue, 19 Apr 2016 07:04:47 -0700 Subject: [PATCH] Potential division by zero in RtpToNtpMs() in rtp_to_ntp.cc. CalculateFrequency() results in zero frequency (floating point) if the RTP timestamps in the RTCP list are equal. Added check in UpdateRtcpList to not insert RTCP SR with the same RTP timestamp. BUG=webrtc:5780 Review URL: https://codereview.webrtc.org/1891703002 Cr-Commit-Position: refs/heads/master@{#12429} --- webrtc/system_wrappers/source/rtp_to_ntp.cc | 5 ++- .../source/rtp_to_ntp_unittest.cc | 40 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/webrtc/system_wrappers/source/rtp_to_ntp.cc b/webrtc/system_wrappers/source/rtp_to_ntp.cc index 706d861b55..6504737fd5 100644 --- a/webrtc/system_wrappers/source/rtp_to_ntp.cc +++ b/webrtc/system_wrappers/source/rtp_to_ntp.cc @@ -71,8 +71,9 @@ bool UpdateRtcpList(uint32_t ntp_secs, for (RtcpList::iterator it = rtcp_list->begin(); it != rtcp_list->end(); ++it) { - if (measurement.ntp_secs == (*it).ntp_secs && - measurement.ntp_frac == (*it).ntp_frac) { + if ((measurement.ntp_secs == (*it).ntp_secs && + measurement.ntp_frac == (*it).ntp_frac) || + (measurement.rtp_timestamp == (*it).rtp_timestamp)) { // This RTCP has already been added to the list. return true; } diff --git a/webrtc/system_wrappers/source/rtp_to_ntp_unittest.cc b/webrtc/system_wrappers/source/rtp_to_ntp_unittest.cc index d2929f5cbc..78e4a52716 100644 --- a/webrtc/system_wrappers/source/rtp_to_ntp_unittest.cc +++ b/webrtc/system_wrappers/source/rtp_to_ntp_unittest.cc @@ -135,4 +135,44 @@ TEST(WrapAroundTests, OldRtp_OldRtcpWrapped) { int64_t timestamp_in_ms = -1; EXPECT_FALSE(RtpToNtpMs(timestamp, rtcp, ×tamp_in_ms)); } + +TEST(UpdateRtcpListTests, InjectRtcpSrWithEqualNtp) { + RtcpList rtcp; + uint32_t ntp_sec = 0; + uint32_t ntp_frac = 2; + uint32_t timestamp = 0x12345678; + + bool new_sr; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_TRUE(new_sr); + + ++timestamp; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_FALSE(new_sr); +} + +TEST(UpdateRtcpListTests, InjectRtcpSrWithEqualTimestamp) { + RtcpList rtcp; + uint32_t ntp_sec = 0; + uint32_t ntp_frac = 2; + uint32_t timestamp = 0x12345678; + + bool new_sr; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_TRUE(new_sr); + + ++ntp_frac; + EXPECT_TRUE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); + EXPECT_FALSE(new_sr); +} + +TEST(UpdateRtcpListTests, InjectRtcpSrWithZeroNtpFails) { + RtcpList rtcp; + uint32_t ntp_sec = 0; + uint32_t ntp_frac = 0; + uint32_t timestamp = 0x12345678; + + bool new_sr; + EXPECT_FALSE(UpdateRtcpList(ntp_sec, ntp_frac, timestamp, &rtcp, &new_sr)); +} }; // namespace webrtc