gst-libs/gst/rtp/gstbasertpdepayload.c: Validate the RTP packet before further processing it. It's just too dangerous...

Original commit message from CVS:
* gst-libs/gst/rtp/gstbasertpdepayload.c:
(gst_base_rtp_depayload_chain):
Validate the RTP packet before further processing it. It's just too
dangerous to accept random packets and people are not forced to use a
jitterbuffer or session manager to filter out the bad packets.
* gst-libs/gst/rtp/gstrtpbuffer.c:
(gst_rtp_buffer_set_extension_data),
(gst_rtp_buffer_get_payload_subbuffer):
Small cleanups.
When setting extension data in a buffer that is too small, we fail and
we should not set the extension bit.
Change GST_WARNINGS into g_warning because they really are
programming errors.
* tests/check/libs/rtp.c: (GST_START_TEST):
Catch the g_warnings now in the unit tests and that fact that failing to
set extension data left the extension bit untouched.

ChangeLog

@@ -1,3 +1,24 @@
+2008-05-14  Wim Taymans  <wim.taymans@collabora.co.uk>
+
+	* gst-libs/gst/rtp/gstbasertpdepayload.c:
+	(gst_base_rtp_depayload_chain):
+	Validate the RTP packet before further processing it. It's just too
+	dangerous to accept random packets and people are not forced to use a
+	jitterbuffer or session manager to filter out the bad packets.
+
+	* gst-libs/gst/rtp/gstrtpbuffer.c:
+	(gst_rtp_buffer_set_extension_data),
+	(gst_rtp_buffer_get_payload_subbuffer):
+	Small cleanups.
+	When setting extension data in a buffer that is too small, we fail and
+	we should not set the extension bit.
+	Change GST_WARNINGS into g_warning because they really are
+	programming errors.
+
+	* tests/check/libs/rtp.c: (GST_START_TEST):
+	Catch the g_warnings now in the unit tests and that fact that failing to
+	set extension data left the extension bit untouched.
+
 2008-05-14  Tim-Philipp Müller  <tim.muller at collabora co uk>
 
 	* gst/audioresample/gstaudioresample.c: (gst_audioresample_init):

common

@@ -1 +1 @@
-Subproject commit 2d9c09df0fe4ad3f570fea9f649cfc6c4511080d
+Subproject commit 0823ac6b46b7332109bbc5f1ef40e24e648fae51

gst-libs/gst/rtp/gstbasertpdepayload.c

@@ -258,6 +258,9 @@ gst_base_rtp_depayload_chain (GstPad * pad, GstBuffer * in)
 
   filter = GST_BASE_RTP_DEPAYLOAD (GST_OBJECT_PARENT (pad));
 
+  if (!gst_rtp_buffer_validate (in))
+    goto invalid_buffer;
+
   priv = filter->priv;
   priv->discont = GST_BUFFER_IS_DISCONT (in);
 
@@ -285,6 +288,16 @@ gst_base_rtp_depayload_chain (GstPad * pad, GstBuffer * in)
   gst_buffer_unref (in);
 
   return ret;
+
+  /* ERRORS */
+invalid_buffer:
+  {
+    /* this is not fatal but should be filtered earlier */
+    GST_ELEMENT_WARNING (filter, STREAM, DECODE, (NULL),
+        ("Received invalid RTP payload, dropping"));
+    gst_buffer_unref (in);
+    return GST_FLOW_OK;
+  }
 }
 
 static gboolean

gst-libs/gst/rtp/gstrtpbuffer.c

@@ -655,24 +655,31 @@ gst_rtp_buffer_set_extension_data (GstBuffer * buffer, guint16 bits,
   g_return_val_if_fail (GST_IS_BUFFER (buffer), FALSE);
   g_return_val_if_fail (GST_BUFFER_DATA (buffer) != NULL, FALSE);
 
-  gst_rtp_buffer_set_extension (buffer, TRUE);
+  /* check if the buffer is big enough to hold the extension */
   min_size =
       GST_RTP_HEADER_LEN + GST_RTP_HEADER_CSRC_SIZE (buffer) + 4 +
       length * sizeof (guint32);
+  if (G_UNLIKELY (min_size > GST_BUFFER_SIZE (buffer)))
+    goto too_small;
 
-  if (min_size > GST_BUFFER_SIZE (buffer)) {
-    GST_WARNING_OBJECT (buffer,
-        "rtp buffer too small: need more than %d bytes but only have %d bytes",
-        min_size, GST_BUFFER_SIZE (buffer));
-    return FALSE;
-  }
+  /* now we can set the extension bit */
+  gst_rtp_buffer_set_extension (buffer, TRUE);
 
-  data =
-      GST_BUFFER_DATA (buffer) + GST_RTP_HEADER_LEN +
+  data = GST_BUFFER_DATA (buffer) + GST_RTP_HEADER_LEN +
       GST_RTP_HEADER_CSRC_SIZE (buffer);
   GST_WRITE_UINT16_BE (data, bits);
   GST_WRITE_UINT16_BE (data + 2, length);
+
   return TRUE;
+
+  /* ERRORS */
+too_small:
+  {
+    g_warning
+        ("rtp buffer too small: need more than %d bytes but only have %d bytes",
+        min_size, GST_BUFFER_SIZE (buffer));
+    return FALSE;
+  }
 }
 
 /**
@@ -920,10 +927,8 @@ gst_rtp_buffer_get_payload_subbuffer (GstBuffer * buffer, guint offset,
 
   plen = gst_rtp_buffer_get_payload_len (buffer);
   /* we can't go past the length */
-  if (G_UNLIKELY (offset >= plen)) {
-    GST_WARNING ("offset=%u should be less then plen=%u", offset, plen);
-    return (NULL);
-  }
+  if (G_UNLIKELY (offset >= plen))
+    goto wrong_offset;
 
   /* apply offset */
   poffset = gst_rtp_buffer_get_header_len (buffer) + offset;
@@ -934,6 +939,13 @@ gst_rtp_buffer_get_payload_subbuffer (GstBuffer * buffer, guint offset,
     plen = len;
 
   return gst_buffer_create_sub (buffer, poffset, plen);
+
+  /* ERRORS */
+wrong_offset:
+  {
+    g_warning ("offset=%u should be less then plen=%u", offset, plen);
+    return NULL;
+  }
 }
 
 /**

tests/check/libs/rtp.c

@@ -144,8 +144,9 @@ GST_START_TEST (test_rtp_buffer_set_extension_data)
   data = GST_BUFFER_DATA (buf);
 
   /* should be impossible to set the extension data */
-  fail_unless (gst_rtp_buffer_set_extension_data (buf, 0, 4) == FALSE);
-  fail_unless (gst_rtp_buffer_get_extension (buf) == TRUE);
+  ASSERT_WARNING (fail_unless (gst_rtp_buffer_set_extension_data (buf, 0,
+              4) == FALSE));
+  fail_unless (gst_rtp_buffer_get_extension (buf) == FALSE);
 
   /* should be possible to set the extension data */
   fail_unless (gst_rtp_buffer_set_extension_data (buf, 270, 0) == TRUE);