GNOME Bugzilla – Bug 655727
rtp: segfault in gst_rtcp_packet_get_rb()
Last modified: 2013-08-28 13:57:33 UTC
Created attachment 192991 [details] [review] Segfault gst_rtcp_packet_get_rb hello, I found a segfault in the function gst_rtcp_packet_get_rb at line 1002, when GST_READ_UINT32_BE (data) is called; it seems that offsetting the pointer to data can cause a buffer overflow (e.g. *data += 24) in some conditions. I noticed that in my case (using and Elphel camera where buffer size is always 28) packet->type is never GST_RTCP_TYPE_RR, which causes a systematic 24 offset; added to other offset operations, the total offset sometimes is > buffer->size, which causes (IMO) the GST_READ_UINT32_BE segfault. How should this bug be fixed ? Am i correct in analyzing the behaviour ? Any ideas ? Pipeline : gst-launch rtspsrc location=camera protocols=0x00000001 latency=50 ! rtpjpegdepay ! jpegdec ! videoscale ! videorate ! "video/x-raw-yuv, format=(fourcc)I420, width=(int)2592, height=(int)816, framerate=(fraction)25/1" ! jpegenc ! matroskamux ! filesink location=test_jpeg.mkv Details (pipeline, full backtrace) here: http://pastebin.com/g3t8YLBg For the moment, i made a patch for correct this bug.
Could you please attach a patch with correct author information and without unrelated whitespace changes? Also the added check for the size looks suspicious, can you really assume that the offset is 4 if the packet size is too small?
I will correct the patch. In the gst_rtcp_packet_get_rb function the offset can be 28 or 48. There is no check for buffer under 28, so i will add one. If the buffer is 28 the offset must be 4 and not 24 that was my problem. i can try to implement a functionnality that calculate the offset with buffer from 28 to 48.
Created attachment 193170 [details] [review] Bug fix in gst_rtcp_packet_get_rb I followed the official documentation (http://gstreamer.freedesktop.org/wiki/SubmittingPatches) which states that gst-indent should be used on patches before submitting (which i did), which removes all whitespaces. git refuses commits if whitespaces aren't removed. What should i do ? I updated the author information.
Created attachment 193548 [details] [review] Bug-in-gst_rtcp_packet_add_rb-overflow-data i made patch without unrelated whitespace changes. I think the patch is good now.
This patch looks weird, what is wrong with the RTCP packet? can you maybe make a hexdump of it?
Wim, you pushed: commit ca1dac69826d47d6700fd6c22d7ffbb3f638dfaa Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Mon Aug 26 11:47:40 2013 +0200 rtcpbuffer: do additional packet checks Check the packet size and avoid crashing on malformed packets. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=655727 - can this be closed then?
yes
Cherry-picked into 1.0 as well.