srtpdec: Fix a use-after-free buffer issue
authorPhilippe Normand <philn@igalia.com>
Sun, 22 Dec 2024 14:00:07 +0000 (15:00 +0100)
committerPhilippe Normand <philn@igalia.com>
Sun, 22 Dec 2024 14:00:07 +0000 (15:00 +0100)
The gst_srtp_dec_decode_buffer() function modifies the input buffer after making
it writable, so the pointer might change as well, depending on the refcount of
the buffer.

This issue was detected using a netsim element upstream of the decoder in a
WebRTC pipeline.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8198>

subprojects/gst-plugins-bad/ext/srtp/gstsrtpdec.c

index 0247d0098d6bbe817c26f1a330062559bf436453..7bdd3d95039fd6b2db63b05a62024c1425109634 100644 (file)
@@ -1339,10 +1339,11 @@ err:
 }
 
 /*
- * This function should be called while holding the filter lock
+ * This function should be called while holding the filter lock.
+ * The decoded buffer is stored in-place of the input @buf.
  */
 static gboolean
-gst_srtp_dec_decode_buffer (GstSrtpDec * filter, GstPad * pad, GstBuffer * buf,
+gst_srtp_dec_decode_buffer (GstSrtpDec * filter, GstPad * pad, GstBuffer ** buf,
     gboolean is_rtcp, guint32 ssrc)
 {
   GstMapInfo map;
@@ -1350,14 +1351,16 @@ gst_srtp_dec_decode_buffer (GstSrtpDec * filter, GstPad * pad, GstBuffer * buf,
   gint size;
   GstSrtpDecSsrcStream *stream;
 
+  g_return_val_if_fail (GST_IS_BUFFER (*buf), FALSE);
+
   GST_LOG_OBJECT (pad, "Received %s buffer of size %" G_GSIZE_FORMAT
-      " with SSRC = %u", is_rtcp ? "RTCP" : "RTP", gst_buffer_get_size (buf),
+      " with SSRC = %u", is_rtcp ? "RTCP" : "RTP", gst_buffer_get_size (*buf),
       ssrc);
   filter->recv_count++;
   /* Change buffer to remove protection */
-  buf = gst_buffer_make_writable (buf);
+  *buf = gst_buffer_make_writable (*buf);
 
-  gst_buffer_map (buf, &map, GST_MAP_READWRITE);
+  gst_buffer_map (*buf, &map, GST_MAP_READWRITE);
   size = map.size;
 
 unprotect:
@@ -1463,13 +1466,13 @@ unprotect:
       stream->recv_drop_count++;
       goto err;
   }
-  gst_buffer_unmap (buf, &map);
-  gst_buffer_set_size (buf, size);
+  gst_buffer_unmap (*buf, &map);
+  gst_buffer_set_size (*buf, size);
   return TRUE;
 
 err:
   filter->recv_drop_count++;
-  gst_buffer_unmap (buf, &map);
+  gst_buffer_unmap (*buf, &map);
   return FALSE;
 }
 
@@ -1498,7 +1501,7 @@ gst_srtp_dec_chain (GstPad * pad, GstObject * parent, GstBuffer * buf,
     goto push_out;
   }
 
-  if (!gst_srtp_dec_decode_buffer (filter, pad, buf, is_rtcp, ssrc)) {
+  if (!gst_srtp_dec_decode_buffer (filter, pad, &buf, is_rtcp, ssrc)) {
     GST_OBJECT_UNLOCK (filter);
     goto drop_buffer;
   }