After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 756653 - srtpdec: buffer validations may fail if SRTP packets have padding flag set
srtpdec: buffer validations may fail if SRTP packets have padding flag set
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.6.0
Other Linux
: Normal normal
: 1.6.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 752705
Blocks:
 
 
Reported: 2015-10-15 16:56 UTC by Miguel París Díaz
Modified: 2016-03-08 05:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rtpdec: skip padding when mapping RTP packets (1.35 KB, patch)
2015-10-15 22:27 UTC, Miguel París Díaz
committed Details | Review

Description Miguel París Díaz 2015-10-15 16:56:11 UTC
Hello,
I have come across that srtpdec may fail when buffers are validated if the SRTP packets have padding flag set.

The problem is that srtpdec is using gst_rtp_buffer_map [1] to verify that the incoming GstBuffer is an RTP packet, but gst_rtp_buffer_map tries to map an encrypted RTP packet (SRTP).
If the padding flag is set [2], it gets the padding len from the last octet of the packet, but this is not the real padding value because all payload (included the padding) is encrypted [3].

I think that a proper solution is add an extra map function for only mapping the header of the RTP packet and use it into srtpdec.
This will solve the problem and, in addition, it will be more efficient that mapping the whole RTP buffer when only header is relevant for the user.

Refs:
[1] http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/ext/srtp/gstsrtpdec.c?h=1.6#n628
[2] http://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst-libs/gst/rtp/gstrtpbuffer.c?h=1.6#n403
[3] http://tools.ietf.org/html/rfc3711#section-3.1
Comment 1 Sebastian Dröge (slomo) 2015-10-15 19:40:16 UTC
See bug #752705 for a similar problem. That could also help here
Comment 2 Miguel París Díaz 2015-10-15 22:27:05 UTC
Created attachment 313407 [details] [review]
rtpdec: skip padding when mapping RTP packets
Comment 3 Miguel París Díaz 2015-10-15 22:30:39 UTC
I am uploading a patch based on GST_RTP_BUFFER_MAP_FLAG_SKIP_PADDING.
This solve the problem, and for now it could be enough.

On the other hand, we should start thinking about performance and take into account that in many cases we are mapping all RTP info without needed it.
Related issue: #754189
Comment 4 Sebastian Dröge (slomo) 2015-10-16 06:18:12 UTC
commit f19a789b685e9fb836275d7323d4dea7004084c2
Author: Miguel París Díaz <mparisdiaz@gmail.com>
Date:   Fri Oct 16 00:23:56 2015 +0200

    srtpdec: skip padding when mapping RTP packets
    
    https://bugzilla.gnome.org/show_bug.cgi?id=756653