rtcpbuffer: Fix validation of packets with padding

The padding (if any) is included in the length of the last packet, see
RFC 3550.

Section 6.4.1:
   padding (P): 1 bit
      If the padding bit is set, this individual RTCP packet contains
      some additional padding octets at the end which are not part of
      the control information but are included in the length field. The
      last octet of the padding is a count of how many padding octets
      should be ignored, including itself (it will be a multiple of
      four).

Section A.2:
   *  The padding bit (P) should be zero for the first packet of a
      compound RTCP packet because padding should only be applied, if it
      is needed, to the last packet.

   *  The length fields of the individual RTCP packets must add up to
      the overall length of the compound RTCP packet as received.

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

diff --git a/gst-libs/gst/rtp/gstrtcpbuffer.c b/gst-libs/gst/rtp/gstrtcpbuffer.c
index 7d6b541..e5e4ca7 100644 (file)
--- a/gst-libs/gst/rtp/gstrtcpbuffer.c
+++ b/gst-libs/gst/rtp/gstrtcpbuffer.c
@@ -129,24 +129,28 @@ gst_rtcp_buffer_validate_data_internal (guint8 * data, guint len,
     if (data_len < 4)
       break;
 
+    /* padding only allowed on last packet */
+    if (padding)
+      break;
+
     /* check version of new packet */
     version = data[0] & 0xc0;
     if (version != (GST_RTCP_VERSION << 6))
       goto wrong_version;
 
-    /* padding only allowed on last packet */
-    if ((padding = data[0] & 0x20))
-      break;
+    /* check padding of new packet */
+    if (data[0] & 0x20) {
+      padding = TRUE;
+      /* last byte of padding contains the number of padded bytes including
+       * itself. must be a multiple of 4, but cannot be 0. */
+      pad_bytes = data[data_len - 1];
+      if (pad_bytes == 0 || (pad_bytes & 0x3))
+        goto wrong_padding;
+    }
   }
-  if (data_len > 0) {
-    /* some leftover bytes, check padding */
-    if (!padding)
-      goto wrong_length;
-
-    /* get padding */
-    pad_bytes = data[data_len - 1];
-    if (data_len != pad_bytes)
-      goto wrong_padding;
+  if (data_len != 0) {
+    /* some leftover bytes */
+    goto wrong_length;
   }
   return TRUE;
 

diff --git a/tests/check/libs/rtp.c b/tests/check/libs/rtp.c
index 164a2f6..77b3d19 100644 (file)
--- a/tests/check/libs/rtp.c
+++ b/tests/check/libs/rtp.c
@@ -796,6 +796,164 @@ GST_START_TEST (test_rtcp_reduced_buffer)
 
 GST_END_TEST;
 
+
+GST_START_TEST (test_rtcp_validate_with_padding)
+{
+  /* Compound packet with padding in the last packet. Padding is included in
+   * the length of the last packet. */
+  guint8 rtcp_pkt[] = {
+    0x80, 0xC9, 0x00, 0x07, /* Type RR, length = 7 */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x4d, 0x16, 0xaf, 0x14,
+    0x10, 0x1f, 0xd9, 0x91,
+    0x0f, 0xb7, 0x50, 0x88,
+    0x3b, 0x79, 0x31, 0x50,
+    0xbe, 0x19, 0x12, 0xa8,
+    0xbb, 0xce, 0x9e, 0x3e,
+    0xA0, 0xCA, 0x00, 0x0A, /* P=1, Type SDES, length = 10 (includes padding) */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x01, 0x0F, 0x00, 0x00, /* Type 1 (CNAME), length 15 */
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x02, 0x09, 0x00, /* Type 2 (NAME), length 9 */
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, /* Type 0 (no length, 2 unused bytes) */
+    0x00, 0x00, 0x00, 0x04  /* RTCP padding */
+  };
+
+  fail_unless (gst_rtcp_buffer_validate_data (rtcp_pkt, sizeof (rtcp_pkt)));
+}
+GST_END_TEST;
+
+GST_START_TEST (test_rtcp_validate_with_padding_wrong_padlength)
+{
+  /* Compound packet with padding in the last packet. Padding is included in
+   * the length of the last packet. */
+  guint8 rtcp_pkt[] = {
+    0x80, 0xC9, 0x00, 0x07, /* Type RR, length = 7 */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x4d, 0x16, 0xaf, 0x14,
+    0x10, 0x1f, 0xd9, 0x91,
+    0x0f, 0xb7, 0x50, 0x88,
+    0x3b, 0x79, 0x31, 0x50,
+    0xbe, 0x19, 0x12, 0xa8,
+    0xbb, 0xce, 0x9e, 0x3e,
+    0xA0, 0xCA, 0x00, 0x0A, /* P=1, Type SDES, length = 10 (includes padding) */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x01, 0x0F, 0x00, 0x00, /* Type 1 (CNAME), length 15 */
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x02, 0x09, 0x00, /* Type 2 (NAME), length 9 */
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, /* Type 0 (no length, 2 unused bytes) */
+    0x00, 0x00, 0x00, 0x03  /* RTCP padding (wrong length) */
+  };
+
+  fail_if (gst_rtcp_buffer_validate_data (rtcp_pkt, sizeof (rtcp_pkt)));
+}
+GST_END_TEST;
+
+GST_START_TEST (test_rtcp_validate_with_padding_excluded_from_length)
+{
+  /* Compound packet with padding in the last packet. Padding is not included
+   * in the length. */
+  guint8 rtcp_pkt[] = {
+    0x80, 0xC9, 0x00, 0x07, /* Type RR, length = 7 */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x4d, 0x16, 0xaf, 0x14,
+    0x10, 0x1f, 0xd9, 0x91,
+    0x0f, 0xb7, 0x50, 0x88,
+    0x3b, 0x79, 0x31, 0x50,
+    0xbe, 0x19, 0x12, 0xa8,
+    0xbb, 0xce, 0x9e, 0x3e,
+    0xA0, 0xCA, 0x00, 0x09, /* P=1, Type SDES, length = 9 (excludes padding) */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x01, 0x0F, 0x00, 0x00, /* Type 1 (CNAME), length 15 */
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x02, 0x09, 0x00, /* Type 2 (NAME), length 9 */
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, /* Type 0 (no length, 2 unused bytes) */
+    0x00, 0x00, 0x00, 0x04  /* RTCP padding */
+  };
+
+  fail_if (gst_rtcp_buffer_validate_data (rtcp_pkt, sizeof (rtcp_pkt)));
+}
+GST_END_TEST;
+
+GST_START_TEST (test_rtcp_validate_with_padding_set_in_first_packet)
+{
+  /* Compound packet with padding in the last packet but with the pad
+     bit set on first packet */
+  guint8 rtcp_pkt[] = {
+    0xA0, 0xC9, 0x00, 0x07, /* P=1, Type RR, length = 7 */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x4d, 0x16, 0xaf, 0x14,
+    0x10, 0x1f, 0xd9, 0x91,
+    0x0f, 0xb7, 0x50, 0x88,
+    0x3b, 0x79, 0x31, 0x50,
+    0xbe, 0x19, 0x12, 0xa8,
+    0xbb, 0xce, 0x9e, 0x3e,
+    0x80, 0xCA, 0x00, 0x0a, /* Type SDES, length = 10 (include padding) */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x01, 0x0F, 0x00, 0x00, /* Type 1 (CNAME), length 15 */
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x02, 0x09, 0x00, /* Type 2 (NAME), length 9 */
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, /* Type 0 (no length, 2 unused bytes) */
+    0x00, 0x00, 0x00, 0x04  /* RTCP padding */
+  };
+
+  fail_if (gst_rtcp_buffer_validate_data (rtcp_pkt, sizeof (rtcp_pkt)));
+}
+GST_END_TEST;
+
+GST_START_TEST (test_rtcp_validate_reduced_without_padding)
+{
+  /* Reduced size packet without padding */
+  guint8 rtcp_pkt[] = {
+    0x80, 0xcd, 0x00, 0x07, /* Type FB, length = 8 */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x4d, 0x16, 0xaf, 0x14,
+    0x10, 0x1f, 0xd9, 0x91,
+    0x0f, 0xb7, 0x50, 0x88,
+    0x3b, 0x79, 0x31, 0x50,
+    0xbe, 0x19, 0x12, 0xa8,
+    0xbb, 0xce, 0x9e, 0x3e,
+  };
+
+  fail_unless (gst_rtcp_buffer_validate_data_reduced (rtcp_pkt, sizeof (rtcp_pkt)));
+}
+GST_END_TEST;
+
+GST_START_TEST (test_rtcp_validate_reduced_with_padding)
+{
+  /* Reduced size packet with padding. */
+  guint8 rtcp_pkt[] = {
+    0xA0, 0xcd, 0x00, 0x08, /* P=1, Type FB, length = 8 */
+    0x97, 0x6d, 0x21, 0x6a,
+    0x4d, 0x16, 0xaf, 0x14,
+    0x10, 0x1f, 0xd9, 0x91,
+    0x0f, 0xb7, 0x50, 0x88,
+    0x3b, 0x79, 0x31, 0x50,
+    0xbe, 0x19, 0x12, 0xa8,
+    0xbb, 0xce, 0x9e, 0x3e,
+    0x00, 0x00, 0x00, 0x04  /* RTCP padding */
+  };
+
+  fail_if (gst_rtcp_buffer_validate_data_reduced (rtcp_pkt, sizeof (rtcp_pkt)));
+}
+GST_END_TEST;
+
 GST_START_TEST (test_rtp_ntp64_extension)
 {
   GstBuffer *buf;
@@ -1027,6 +1185,12 @@ rtp_suite (void)
 
   tcase_add_test (tc_chain, test_rtcp_buffer);
   tcase_add_test (tc_chain, test_rtcp_reduced_buffer);
+  tcase_add_test (tc_chain, test_rtcp_validate_with_padding);
+  tcase_add_test (tc_chain, test_rtcp_validate_with_padding_wrong_padlength);
+  tcase_add_test (tc_chain, test_rtcp_validate_with_padding_excluded_from_length);
+  tcase_add_test (tc_chain, test_rtcp_validate_with_padding_set_in_first_packet);
+  tcase_add_test (tc_chain, test_rtcp_validate_reduced_without_padding);
+  tcase_add_test (tc_chain, test_rtcp_validate_reduced_with_padding);
   tcase_add_test (tc_chain, test_rtp_ntp64_extension);
   tcase_add_test (tc_chain, test_rtp_ntp56_extension);