GNOME Bugzilla – Bug 743414
rtpvp8: undefined behaviour. division by zero
Last modified: 2015-01-26 10:37:32 UTC
Created attachment 295288 [details] [review] proposed patch gst_rtp_buffer_calc_payload_len () can return zero if packet length is smaller than header length. It is unlikely but this function checks for this and returns 0, then if it does, max_paylen will become zero and will be the divisor in the next line. list = gst_buffer_list_new_sized ((size / max_paylen) + 1); Which would make _handle_buffer() of rtpvp8 have undefined behaviour.
Not completely sure about the solution. Posting it here for review/discussion. If anybody likes it, please mark it as accepted.
I don't think this can happen or make sense to add "just in case". Minimum mtu is 28 (coverity probably doesn't know that), and usually more like ~1400. Max vp8_hdr_len is 4, so smallest packet_len passed to gst_rtp_buffer_calc_payload_len() is 24. csrc_count and pad_len are always 0 here, and GST_RTP_HEADER_LEN is 12. (>=24) < 12 -> always false.
I understand. I can mark the coverity issue as a false positive. Does this mean the following check in gst_rtp_buffer_calc_payload_len() is never going to be True?: if (packet_len < GST_RTP_HEADER_LEN + (csrc_count * sizeof (guint32)) + pad_len) return 0; gst-plugins-base/gst-libs/gst/rtp/gstrtpbuffer.c:292 Should it be removed or does it serve a purpose?
It does serve a purpose in general, just not in this particular case. I don't think it should be removed.
I will close this bug then. Thanks for looking into it.