commit 271195f33686d6fc1aba51c0d43d76dc6839c4dd
author Danil Chapovalov <danilchap@webrtc.org> Mon Feb 11 10:30:03 2019
committer Commit Bot <commit-bot@chromium.org> Mon Feb 11 13:21:55 2019
tree f36be800e091a3ae1277180a62d36a48dfffe8f8
parent 501bfba0cb5f9649956451dd0d14f56b62af9af4

Fix potential crash when building rtx packet

rtx packet may have addition extension (mid) and may use different
header size for extension (e.g. if repaired rtp stream id registered
to larger id than rtp stream id)

As a result rtx packet size calculation as orginial size + 2 bytes in
some scenarious may be incorrect. This chenage avoids crash in that cases.

Bug: None
Change-Id: I620d95e0592d6bdac0d3623b2675a49fc2177580
Reviewed-on: https://webrtc-review.googlesource.com/c/122180
Reviewed-by: Erik Varga <erikvarga@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26635}

modules/rtp_rtcp/source/rtp_sender.cc

diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index ad7e631..73d6413 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -1022,16 +1022,8 @@
   return sequence_number_;
 }
 
-static std::unique_ptr<RtpPacketToSend> CreateRtxPacket(
-    const RtpPacketToSend& packet,
-    RtpHeaderExtensionMap* extension_map) {
-  RTC_DCHECK(extension_map);
-  // TODO(danilchap): Create rtx packet with extra capacity for SRTP
-  // when transport interface would be updated to take buffer class.
-  size_t packet_size = packet.size() + kRtxHeaderSize;
-  std::unique_ptr<RtpPacketToSend> rtx_packet =
-      absl::make_unique<RtpPacketToSend>(extension_map, packet_size);
-
+static void CopyHeaderAndExtensionsToRtxPacket(const RtpPacketToSend& packet,
+                                               RtpPacketToSend* rtx_packet) {
   // Set the relevant fixed packet headers. The following are not set:
   // * Payload type - it is replaced in rtx packets.
   // * Sequence number - RTX has a separate sequence numbering.
@@ -1075,14 +1067,11 @@
 
     std::memcpy(destination.begin(), source.begin(), destination.size());
   }
-
-  return rtx_packet;
 }
 
 std::unique_ptr<RtpPacketToSend> RTPSender::BuildRtxPacket(
     const RtpPacketToSend& packet) {
-  std::unique_ptr<RtpPacketToSend> rtx_packet =
-      CreateRtxPacket(packet, &rtp_header_extension_map_);
+  std::unique_ptr<RtpPacketToSend> rtx_packet;
 
   // Add original RTP header.
   {
@@ -1096,6 +1085,10 @@
     auto kv = rtx_payload_type_map_.find(packet.PayloadType());
     if (kv == rtx_payload_type_map_.end())
       return nullptr;
+
+    rtx_packet = absl::make_unique<RtpPacketToSend>(&rtp_header_extension_map_,
+                                                    max_packet_size_);
+
     rtx_packet->SetPayloadType(kv->second);
 
     // Replace sequence number.
@@ -1104,6 +1097,8 @@
     // Replace SSRC.
     rtx_packet->SetSsrc(*ssrc_rtx_);
 
+    CopyHeaderAndExtensionsToRtxPacket(packet, rtx_packet.get());
+
     // The spec indicates that it is possible for a sender to stop sending mids
     // once the SSRCs have been bound on the receiver. As a result the source
     // rtp packet might not have the MID header extension set.
@@ -1119,10 +1114,13 @@
       // rtx_packet->SetExtension<RepairedRtpStreamId>(rid_);
     }
   }
+  RTC_DCHECK(rtx_packet);
 
   uint8_t* rtx_payload =
       rtx_packet->AllocatePayload(packet.payload_size() + kRtxHeaderSize);
-  RTC_DCHECK(rtx_payload);
+  if (rtx_payload == nullptr)
+    return nullptr;
+
   // Add OSN (original sequence number).
   ByteWriter<uint16_t>::WriteBigEndian(rtx_payload, packet.SequenceNumber());