h264parser: fix stack smashing

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 <git@andred.net>

diff --git a/gst-libs/gst/codecparsers/gsth264parser.c b/gst-libs/gst/codecparsers/gsth264parser.c
index 4df2bad0d2c58e44d0ca06c35503ad04e3d8ecac..ae6cb54f7547d98e7c0c09cb6d1cf0d2de7ef1ec 100644 (file)
--- 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;