commit 3c0d006c03c4118df4700d17bf540615f2c634ed
author Haakon Sporsheim <haakon@pexip.com> Fri Aug 25 11:57:26 2017 +0200
committer Tim-Philipp Müller <tim@centricular.com> Sat Dec 02 13:58:34 2017 +0000
tree 6b025bc80d9ec103e1f508335823ba5dccf92fc9
parent b4ca81591cb76045c98ac1061eae0b200a3ea9f9

rtpsession: Handle zero length feedback packets

https://bugzilla.gnome.org/show_bug.cgi?id=791074

gst/rtpmanager/rtpsession.c

diff --git a/gst/rtpmanager/rtpsession.c b/gst/rtpmanager/rtpsession.c
index 7e63e1b..0b99d23 100644
--- a/gst/rtpmanager/rtpsession.c
+++ b/gst/rtpmanager/rtpsession.c
@@ -2709,20 +2709,31 @@
 rtp_session_process_feedback (RTPSession * sess, GstRTCPPacket * packet,
     RTPPacketInfo * pinfo, GstClockTime current_time)
 {
-  GstRTCPType type = gst_rtcp_packet_get_type (packet);
-  GstRTCPFBType fbtype = gst_rtcp_packet_fb_get_type (packet);
-  guint32 sender_ssrc = gst_rtcp_packet_fb_get_sender_ssrc (packet);
-  guint32 media_ssrc = gst_rtcp_packet_fb_get_media_ssrc (packet);
-  guint8 *fci_data = gst_rtcp_packet_fb_get_fci (packet);
-  guint fci_length = 4 * gst_rtcp_packet_fb_get_fci_length (packet);
+  GstRTCPType type;
+  GstRTCPFBType fbtype;
+  guint32 sender_ssrc, media_ssrc;
+  guint8 *fci_data;
+  guint fci_length;
   RTPSource *src;
 
+  /* The feedback packet must include both sender SSRC and media SSRC */
+  if (packet->length < 2)
+    return;
+
+  type = gst_rtcp_packet_get_type (packet);
+  fbtype = gst_rtcp_packet_fb_get_type (packet);
+  sender_ssrc = gst_rtcp_packet_fb_get_sender_ssrc (packet);
+  media_ssrc = gst_rtcp_packet_fb_get_media_ssrc (packet);
+
   src = find_source (sess, media_ssrc);
 
   /* skip non-bye packets for sources that are marked BYE */
   if (sess->scheduled_bye && src && RTP_SOURCE_IS_MARKED_BYE (src))
     return;
 
+  fci_data = gst_rtcp_packet_fb_get_fci (packet);
+  fci_length = gst_rtcp_packet_fb_get_fci_length (packet) * sizeof (guint32);
+
   GST_DEBUG ("received feedback %d:%d from %08X about %08X with FCI of "
       "length %d", type, fbtype, sender_ssrc, media_ssrc, fci_length);
 

tests/check/elements/rtpsession.c

diff --git a/tests/check/elements/rtpsession.c b/tests/check/elements/rtpsession.c
index c5d3625..94925ee 100644
--- a/tests/check/elements/rtpsession.c
+++ b/tests/check/elements/rtpsession.c
@@ -683,6 +683,28 @@
 
 GST_END_TEST;
 
+GST_START_TEST (test_illegal_rtcp_fb_packet)
+{
+  SessionHarness *h = session_harness_new ();
+  GstBuffer *buf;
+  /* Zero length RTCP feedback packet (reduced size) */
+  const guint8 rtcp_zero_fb_pkt[] = { 0x8f, 0xce, 0x00, 0x00 };
+
+  g_object_set (h->internal_session, "internal-ssrc", 0xDEADBEEF, NULL);
+
+  buf = gst_buffer_new_and_alloc (sizeof (rtcp_zero_fb_pkt));
+  gst_buffer_fill (buf, 0, rtcp_zero_fb_pkt, sizeof (rtcp_zero_fb_pkt));
+  GST_BUFFER_DTS (buf) = GST_BUFFER_PTS (buf) = G_GUINT64_CONSTANT (0);
+
+  /* Push the packet, this did previously crash because length of packet was
+   * never validated. */
+  fail_unless_equals_int (GST_FLOW_OK, session_harness_recv_rtcp (h, buf));
+
+  session_harness_free (h);
+}
+
+GST_END_TEST;
+
 static Suite *
 rtpsession_suite (void)
 {
@@ -697,6 +719,7 @@
   tcase_add_test (tc_chain, test_dont_lock_on_stats);
   tcase_add_test (tc_chain, test_ignore_suspicious_bye);
 
+  tcase_add_test (tc_chain, test_illegal_rtcp_fb_packet);
   return s;
 }