From 778742963a089daf618fe1d5fbef58cf590cb6ed Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Thu, 12 Jan 2023 09:54:42 +0100 Subject: [PATCH] In remb parser discard bitrate larger than max int64_t Bug: b/265156399 Change-Id: I5bdbd42a8da565972a3c2e976a32a563f3cce6af Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290888 Reviewed-by: Emil Lundmark Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#39082} --- modules/rtp_rtcp/source/rtcp_packet/remb.cc | 10 +++++----- .../rtp_rtcp/source/rtcp_packet/remb_unittest.cc | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_packet/remb.cc b/modules/rtp_rtcp/source/rtcp_packet/remb.cc index 39795fb79c..1389ca7836 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/remb.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/remb.cc @@ -67,15 +67,15 @@ bool Remb::Parse(const CommonHeader& packet) { } ParseCommonFeedback(payload); - uint8_t exponenta = payload[13] >> 2; + uint8_t exponent = payload[13] >> 2; uint64_t mantissa = (static_cast(payload[13] & 0x03) << 16) | ByteReader::ReadBigEndian(&payload[14]); - bitrate_bps_ = (mantissa << exponenta); + bitrate_bps_ = (mantissa << exponent); bool shift_overflow = - (static_cast(bitrate_bps_) >> exponenta) != mantissa; - if (shift_overflow) { + (static_cast(bitrate_bps_) >> exponent) != mantissa; + if (bitrate_bps_ < 0 || shift_overflow) { RTC_LOG(LS_ERROR) << "Invalid remb bitrate value : " << mantissa << "*2^" - << static_cast(exponenta); + << static_cast(exponent); return false; } diff --git a/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc index 391a61de89..c439d9c5f6 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc @@ -101,13 +101,26 @@ TEST(RtcpPacketRembTest, ParseFailsWhenUniqueIdentifierIsNotRemb) { TEST(RtcpPacketRembTest, ParseFailsWhenBitrateDoNotFitIn64bits) { uint8_t packet[kPacketLength]; memcpy(packet, kPacket, kPacketLength); - packet[17] |= 0xfc; // Set exponenta component to maximum of 63. + packet[17] |= 0xfc; // Set exponent component to maximum of 63. packet[19] |= 0x02; // Ensure mantissa is at least 2. Remb remb; EXPECT_FALSE(test::ParseSinglePacket(packet, &remb)); } +TEST(RtcpPacketRembTest, ParseFailsWhenBitrateDoNotFitIn63bits) { + uint8_t packet[kPacketLength]; + memcpy(packet, kPacket, kPacketLength); + packet[17] = 56 << 2; // Set exponent component to 56. + packet[18] = 0; // Set mantissa to 200 > 128 + packet[19] = 200; + + // Result value 200 * 2^56 can't be represented with int64_t and thus should + // be rejected. + Remb remb; + EXPECT_FALSE(test::ParseSinglePacket(packet, &remb)); +} + TEST(RtcpPacketRembTest, ParseFailsWhenSsrcCountMismatchLength) { uint8_t packet[kPacketLength]; memcpy(packet, kPacket, kPacketLength);