Fix oversized rtp extension parsing.

When size of individual one-byte extension span beyound extension block BUG=chromium:645201 R=brandtr@webrtc.org Review URL: https://codereview.webrtc.org/2327743003 . Cr-Commit-Position: refs/heads/master@{#14183}
GetStream · Sep 12, 2016 · a64a2fb · a64a2fb
1 parent 180e452
commit a64a2fb
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 3 deletions.
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet.cc b/webrtc/modules/rtp_rtcp/source/rtp_packet.cc
@@ -11,6 +11,7 @@
 #include "webrtc/modules/rtp_rtcp/source/rtp_packet.h"
 
 #include <cstring>
+#include <utility>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
@@ -397,11 +398,16 @@ bool Packet::ParseBuffer(const uint8_t* buffer, size_t size) {
         }
         uint8_t length =
             1 + (buffer[extension_offset + extensions_size_] & 0xf);
-        extensions_size_ += kOneByteHeaderSize;
+        if (extensions_size_ + kOneByteHeaderSize + length >
+            extensions_capacity) {
+          LOG(LS_WARNING) << "Oversized rtp header extension.";
+          break;
+        }
         if (num_extensions_ >= kMaxExtensionHeaders) {
-          LOG(LS_WARNING) << "Too many extensions.";
-          return false;
+          LOG(LS_WARNING) << "Too many rtp header extensions.";
+          break;
         }
+        extensions_size_ += kOneByteHeaderSize;
         extension_entries_[num_extensions_].type =
             extensions_ ? extensions_->GetType(id)
                         : ExtensionManager::kInvalidType;

diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_packet_unittest.cc
@@ -223,6 +223,29 @@ TEST(RtpPacketTest, ParseWithInvalidSizedExtension) {
               ElementsAreArray(kPayload));
 }
 
+TEST(RtpPacketTest, ParseWithOverSizedExtension) {
+  // clang-format off
+  const uint8_t bad_packet[] = {
+      0x90, kPayloadType, 0x00, kSeqNum,
+      0x65, 0x43, 0x12, 0x78,  // kTimestamp.
+      0x12, 0x34, 0x56, 0x78,  // kSsrc.
+      0xbe, 0xde, 0x00, 0x01,  // Extension of size 1x32bit word.
+      0x00,  // Add a byte of padding.
+            0x12,  // Extension id 1 size (2+1).
+                  0xda, 0x1a  // Only 2 bytes of extension payload.
+  };
+  // clang-format on
+  RtpPacketToSend::ExtensionManager extensions;
+  extensions.Register(TransmissionOffset::kId, 1);
+  RtpPacketReceived packet(&extensions);
+
+  // Parse should ignore bad extension and proceed.
+  EXPECT_TRUE(packet.Parse(bad_packet, sizeof(bad_packet)));
+  int32_t time_offset;
+  // But extracting extension should fail.
+  EXPECT_FALSE(packet.GetExtension<TransmissionOffset>(&time_offset));
+}
+
 TEST(RtpPacketTest, ParseWith2Extensions) {
   RtpPacketToSend::ExtensionManager extensions;
   extensions.Register(kRtpExtensionTransmissionTimeOffset,