Commit 64f9d08d authored by Stian Selnes's avatar Stian Selnes Committed by Olivier Crête

rtph263pay: Fix double free, invalid reads and leak

parent 61bc228a
......@@ -901,14 +901,13 @@ gst_rtp_h263_pay_move_window_right (GstRtpH263PayContext * context, guint n,
return rest_bits;
while (n != 0 || context->win_end == ((*data_end) + 1)) {
//guint8 a = *data;
guint8 b = context->win_end <= *data_end ? *context->win_end : 0;
if (rest_bits == 0) {
if (n > 8) {
context->window = (context->window << 8) | *context->win_end;
context->window = (context->window << 8) | b;
n -= 8;
} else {
context->window =
(context->window << n) | (*context->win_end >> (8 - n));
context->window = (context->window << n) | (b >> (8 - n));
rest_bits = 8 - n;
if (rest_bits == 0)
context->win_end++;
......@@ -916,15 +915,14 @@ gst_rtp_h263_pay_move_window_right (GstRtpH263PayContext * context, guint n,
}
} else {
if (n > rest_bits) {
context->window =
(context->window << rest_bits) | (*context->
win_end & (((guint) pow (2.0, (double) rest_bits)) - 1));
context->window = (context->window << rest_bits) |
(b & (((guint) pow (2.0, (double) rest_bits)) - 1));
n -= rest_bits;
rest_bits = 0;
} else {
context->window =
(context->window << n) | ((*context->win_end & (((guint) pow (2.0,
(double) rest_bits)) - 1)) >> (rest_bits - n));
context->window = (context->window << n) |
((b & (((guint) pow (2.0, (double) rest_bits)) - 1)) >>
(rest_bits - n));
rest_bits -= n;
if (rest_bits == 0)
context->win_end++;
......@@ -1413,7 +1411,7 @@ gst_rtp_h263_pay_mode_B_fragment (GstRtpH263Pay * rtph263pay,
/*---------- MODE B MODE FRAGMENTATION ----------*/
GstRtpH263PayMB *mac = NULL;
GstRtpH263PayMB *mac, *mac0;
guint max_payload_size;
GstRtpH263PayBoundry boundry;
guint mb;
......@@ -1509,7 +1507,7 @@ gst_rtp_h263_pay_mode_B_fragment (GstRtpH263Pay * rtph263pay,
// We are on MB layer
mac = gst_rtp_h263_pay_mb_new (&boundry, 0);
mac = mac0 = gst_rtp_h263_pay_mb_new (&boundry, 0);
for (mb = 0; mb < format_props[context->piclayer->ptype_srcformat][1]; mb++) {
GST_LOG ("================ START MB %d =================", mb);
......@@ -1521,8 +1519,13 @@ gst_rtp_h263_pay_mode_B_fragment (GstRtpH263Pay * rtph263pay,
GST_LOG ("Error decoding MB - sbit: %d", 8 - ebit);
GST_ERROR ("Error decoding in GOB");
gst_rtp_h263_pay_mb_destroy (mac0);
goto decode_error;
}
gst_rtp_h263_pay_mb_destroy (gob->macroblocks[mb]);
gob->macroblocks[mb] = mac;
//If mb_type == stuffing, don't increment the mb address
if (mac->mb_type == 10) {
mb--;
......@@ -1531,8 +1534,6 @@ gst_rtp_h263_pay_mode_B_fragment (GstRtpH263Pay * rtph263pay,
gob->nmacroblocs++;
}
gob->macroblocks[mb] = mac;
if (mac->end >= gob->end) {
GST_LOG ("No more MBs in this GOB");
if (!mac->ebit) {
......@@ -1545,6 +1546,7 @@ gst_rtp_h263_pay_mode_B_fragment (GstRtpH263Pay * rtph263pay,
mac->mba, mac->start, mac->end, mac->length, mac->sbit, mac->ebit);
GST_LOG ("================ END MB %d =================", mb);
}
gst_rtp_h263_pay_mb_destroy (mac0);
mb = 0;
first = 0;
......@@ -1591,11 +1593,10 @@ gst_rtp_h263_pay_mode_B_fragment (GstRtpH263Pay * rtph263pay,
}
/*---------- END OF MODE B FRAGMENTATION ----------*/
gst_rtp_h263_pay_mb_destroy (mac);
return TRUE;
decode_error:
gst_rtp_h263_pay_mb_destroy (mac);
return FALSE;
}
......
......@@ -27,6 +27,21 @@
"application/x-rtp,media=video,encoding-name=H263,clock-rate=90000," \
"payload=" G_STRINGIFY(p)
static gboolean
have_element (const gchar * element_name)
{
GstElement *element;
gboolean ret;
element = gst_element_factory_make (element_name, NULL);
ret = element != NULL;
if (element)
gst_object_unref (element);
return ret;
}
static GstBuffer *
create_rtp_buffer (guint8 * data, gsize size, guint ts, gint seqnum)
{
......@@ -102,6 +117,31 @@ GST_START_TEST (test_h263depay_start_packet_too_small_mode_c)
GST_END_TEST;
GST_START_TEST (test_h263pay_mode_b_snow)
{
/* Payloading one large frame (like snow) is more likely to use mode b and
* trigger issues in valgrind seen previously like double free, invalid read
* etc. */
GstHarness *h;
guint frames = 1;
guint i;
if (!have_element ("avenc_h263"))
return;
h = gst_harness_new_parse (
"avenc_h263 rtp-payload-size=1 ! rtph263pay mtu=1350 ");
gst_harness_add_src_parse (h, "videotestsrc pattern=snow is-live=1 ! "
"capsfilter caps=\"video/x-raw,format=I420,width=176,height=144\"", TRUE);
for (i = 0; i < frames; i++)
gst_harness_push_from_src (h);
fail_unless (gst_harness_buffers_received (h) >= frames);
gst_harness_teardown (h);
}
GST_END_TEST;
static Suite *
rtph263_suite (void)
{
......@@ -113,6 +153,9 @@ rtph263_suite (void)
tcase_add_test (tc_chain, test_h263depay_start_packet_too_small_mode_b);
tcase_add_test (tc_chain, test_h263depay_start_packet_too_small_mode_c);
suite_add_tcase (s, (tc_chain = tcase_create ("h263pay")));
tcase_add_test (tc_chain, test_h263pay_mode_b_snow);
return s;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment