From c73a5e0c545babe50130e7042be8cf92828afa18 Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h Date: Mon, 12 Jan 2015 17:24:52 +0000 Subject: [PATCH] h264parser: fix stack smashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensure that we do not trust the bitstream when filling a table with a fixed max size. Additionally, the code was not quite matching what the spec says: - a value of 3 broke from the loop before adding an entry - an unhandled value did not add an entry The reference algorithm does these things differently (7.3.3.1 in ITU-T Rec. H.264 (05/2003)). This plays (apparently correctly) the original repro file, with no stack smashing. Based on a patch and bug report by André Draszik --- gst-libs/gst/codecparsers/gsth264parser.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/gst-libs/gst/codecparsers/gsth264parser.c b/gst-libs/gst/codecparsers/gsth264parser.c index 4df2bad0d2..ae6cb54f75 100644 --- a/gst-libs/gst/codecparsers/gsth264parser.c +++ b/gst-libs/gst/codecparsers/gsth264parser.c @@ -648,14 +648,17 @@ slice_parse_ref_pic_list_modification_1 (GstH264SliceHdr * slice, GstH264RefPicListModification *entries; guint8 *ref_pic_list_modification_flag, *n_ref_pic_list_modification; guint32 modification_of_pic_nums_idc; + gsize max_entries; guint i = 0; if (list == 0) { entries = slice->ref_pic_list_modification_l0; + max_entries = G_N_ELEMENTS (slice->ref_pic_list_modification_l0); ref_pic_list_modification_flag = &slice->ref_pic_list_modification_flag_l0; n_ref_pic_list_modification = &slice->n_ref_pic_list_modification_l0; } else { entries = slice->ref_pic_list_modification_l1; + max_entries = G_N_ELEMENTS (slice->ref_pic_list_modification_l1); ref_pic_list_modification_flag = &slice->ref_pic_list_modification_flag_l1; n_ref_pic_list_modification = &slice->n_ref_pic_list_modification_l1; } @@ -664,8 +667,6 @@ slice_parse_ref_pic_list_modification_1 (GstH264SliceHdr * slice, if (*ref_pic_list_modification_flag) { while (1) { READ_UE (nr, modification_of_pic_nums_idc); - if (modification_of_pic_nums_idc == 3) - break; if (modification_of_pic_nums_idc == 0 || modification_of_pic_nums_idc == 1) { READ_UE_ALLOWED (nr, entries[i].value.abs_diff_pic_num_minus1, 0, @@ -675,9 +676,12 @@ slice_parse_ref_pic_list_modification_1 (GstH264SliceHdr * slice, } else if (is_mvc && (modification_of_pic_nums_idc == 4 || modification_of_pic_nums_idc == 5)) { READ_UE (nr, entries[i].value.abs_diff_view_idx_minus1); - } else - continue; + } entries[i++].modification_of_pic_nums_idc = modification_of_pic_nums_idc; + if (modification_of_pic_nums_idc == 3) + break; + if (i >= max_entries) + goto error; } } *n_ref_pic_list_modification = i;