qtdemux: move stco, stts, stss and stps atom parsing over to GstByteReader

Make sure we don't read beyond the atom boundary. Note that the code
behaves slightly differently in the corner case where there is not
enough atom data for the specified number of samples (n_samples_time)
in the atom, but still enough data to fill the pre-allocated index of
n_samples entries: before we would just stop parsing the stts data
and continue, whereas now we will likely error out. This should not
be a problem in practice though. We could maintain the old behaviour
by doing reads with a size check inside the loop if needed.

gst/qtdemux/qtdemux.c

@@ -3497,24 +3497,20 @@ static gboolean
 qtdemux_parse_samples (GstQTDemux * qtdemux, QtDemuxStream * stream,
     GNode * stbl)
 {
+  QtAtomParser co_reader;
   QtAtomParser stsz;
   QtAtomParser stsc;
-  GNode *stco;
+  QtAtomParser stts;
-  GNode *co64;
-  GNode *stts;
-  GNode *stss;
-  GNode *stps;
   GNode *ctts;
-  const guint8 *stco_data, *co64_data, *stts_data;
   guint32 sample_size;
   guint32 n_samples;
   guint32 n_samples_per_chunk;
   int sample_index;
-  int n_sample_times;
   QtDemuxSample *samples;
   gint i, j, k;
   int index;
   guint64 timestamp, time;
+  guint co_size;
 
   /* sample to chunk */
   if (!qtdemux_tree_get_child_by_type_full (stbl, FOURCC_stsc, &stsc))
@@ -3525,27 +3521,26 @@ qtdemux_parse_samples (GstQTDemux * qtdemux, QtDemuxStream * stream,
     goto corrupt_file;
 
   /* chunk offsets */
-  stco = qtdemux_tree_get_child_by_type (stbl, FOURCC_stco);
+  if (qtdemux_tree_get_child_by_type_full (stbl, FOURCC_stco, &co_reader))
-  co64 = qtdemux_tree_get_child_by_type (stbl, FOURCC_co64);
+    co_size = sizeof (guint32);
-  if (stco) {
+  else if (qtdemux_tree_get_child_by_type_full (stbl, FOURCC_co64, &co_reader))
-    stco_data = (const guint8 *) stco->data;
+    co_size = sizeof (guint64);
-    co64_data = NULL;
+  else
-  } else {
+    goto corrupt_file;
-    stco_data = NULL;
+
-    if (co64 == NULL)
+  /* sample time */
-      goto corrupt_file;
+  if (!qtdemux_tree_get_child_by_type_full (stbl, FOURCC_stts, &stts))
-    co64_data = (const guint8 *) co64->data;
-  }
-  /* sample time */
-  if (!(stts = qtdemux_tree_get_child_by_type (stbl, FOURCC_stts)))
     goto corrupt_file;
-  stts_data = (const guint8 *) stts->data;
 
   if (!qt_atom_parser_skip (&stsz, 1 + 3) ||
       !qt_atom_parser_get_uint32 (&stsz, &sample_size))
     goto corrupt_file;
 
   if (sample_size == 0 || stream->sampled) {
+    /* skip version, flags, number of entries */
+    if (!gst_byte_reader_skip (&co_reader, 1 + 3 + 4))
+      goto corrupt_file;
+
     if (!qt_atom_parser_get_uint32 (&stsz, &n_samples))
       goto corrupt_file;
 
@@ -3589,6 +3584,7 @@ qtdemux_parse_samples (GstQTDemux * qtdemux, QtDemuxStream * stream,
 
     index = 0;
     for (i = 0; i < n_samples_per_chunk; i++) {
+      QtAtomParser co_chunk;
       guint32 first_chunk, last_chunk;
       guint32 samples_per_chunk;
 
@@ -3615,17 +3611,28 @@ qtdemux_parse_samples (GstQTDemux * qtdemux, QtDemuxStream * stream,
           --last_chunk;
       }
 
+      if (G_UNLIKELY (last_chunk < first_chunk))
+        goto corrupt_file;
+
+      if (last_chunk != G_MAXUINT32) {
+        if (!qt_atom_parser_peek_sub (&co_reader, first_chunk * co_size,
+                (last_chunk - first_chunk) * co_size, &co_chunk))
+          goto corrupt_file;
+      } else {
+        co_chunk = co_reader;
+        if (!qt_atom_parser_skip (&co_chunk, first_chunk * co_size))
+          goto corrupt_file;
+      }
+
       for (j = first_chunk; j < last_chunk; j++) {
         guint64 chunk_offset;
 
-        if (stco) {
+        if (!qt_atom_parser_get_offset (&co_chunk, co_size, &chunk_offset))
-          chunk_offset = QT_UINT32 (stco_data + 16 + j * 4);
+          goto corrupt_file;
-        } else {
+
-          chunk_offset = QT_UINT64 (co64_data + 16 + j * 8);
-        }
         for (k = 0; k < samples_per_chunk; k++) {
-          GST_LOG_OBJECT (qtdemux, "Creating entry %d with offset %lld",
+          GST_LOG_OBJECT (qtdemux, "Creating entry %d with offset %"
-              index, chunk_offset);
+              G_GUINT64_FORMAT, index, chunk_offset);
           samples[index].offset = chunk_offset;
           chunk_offset += samples[index].size;
           index++;
@@ -3635,112 +3642,132 @@ qtdemux_parse_samples (GstQTDemux * qtdemux, QtDemuxStream * stream,
       }
     }
   done2:
+    {
+      guint32 n_sample_times;
 
-    n_sample_times = QT_UINT32 (stts_data + 12);
+      if (!qt_atom_parser_skip (&stts, 4))
-    GST_LOG_OBJECT (qtdemux, "%u timestamp blocks", n_sample_times);
+        goto corrupt_file;
-    timestamp = 0;
+      if (!qt_atom_parser_get_uint32 (&stts, &n_sample_times))
-    stream->min_duration = 0;
+        goto corrupt_file;
-    time = 0;
+      GST_LOG_OBJECT (qtdemux, "%u timestamp blocks", n_sample_times);
-    index = 0;
-    stts_data += 16;
-    for (i = 0; i < n_sample_times; i++) {
-      guint32 n;
-      guint32 duration;
 
-      n = QT_UINT32 (stts_data);
+      /* make sure there's enough data */
-      stts_data += 4;
+      if (qt_atom_parser_get_remaining (&stts) < (n_sample_times * (2 * 4)))
-      duration = QT_UINT32 (stts_data);
+        goto corrupt_file;
-      stts_data += 4;
-      GST_LOG_OBJECT (qtdemux, "block %d, %u timestamps, duration %u ", i, n,
-          duration);
 
-      /* take first duration for fps */
+      timestamp = 0;
-      if (G_UNLIKELY (stream->min_duration == 0))
+      stream->min_duration = 0;
-        stream->min_duration = duration;
+      time = 0;
+      index = 0;
+      for (i = 0; i < n_sample_times; i++) {
+        guint32 n;
+        guint32 duration;
 
-      for (j = 0; j < n; j++) {
+        n = qt_atom_parser_get_uint32_unchecked (&stts);
-        GST_DEBUG_OBJECT (qtdemux,
+        duration = qt_atom_parser_get_uint32_unchecked (&stts);
-            "sample %d: index %d, timestamp %" GST_TIME_FORMAT, index, j,
+        GST_LOG_OBJECT (qtdemux, "block %d, %u timestamps, duration %u ", i, n,
-            GST_TIME_ARGS (timestamp));
+            duration);
 
-        samples[index].timestamp = timestamp;
+        /* take first duration for fps */
-        /* add non-scaled values to avoid rounding errors */
+        if (G_UNLIKELY (stream->min_duration == 0))
-        time += duration;
+          stream->min_duration = duration;
-        timestamp = gst_util_uint64_scale (time, GST_SECOND, stream->timescale);
-        samples[index].duration = timestamp - samples[index].timestamp;
 
-        index++;
+        for (j = 0; j < n; j++) {
-        if (G_UNLIKELY (index >= n_samples))
+          GST_DEBUG_OBJECT (qtdemux,
-          goto done3;
+              "sample %d: index %d, timestamp %" GST_TIME_FORMAT, index, j,
-      }
+              GST_TIME_ARGS (timestamp));
-    }
-    /* fill up empty timestamps with the last timestamp, this can happen when
-     * the last samples do not decode and so we don't have timestamps for them.
-     * We however look at the last timestamp to estimate the track length so we
-     * need something in here. */
-    for (; index < n_samples; index++) {
-      GST_DEBUG_OBJECT (qtdemux, "fill sample %d: timestamp %" GST_TIME_FORMAT,
-          index, GST_TIME_ARGS (timestamp));
-      samples[index].timestamp = timestamp;
-      samples[index].duration = -1;
-    }
-  done3:
 
-    /* sample sync, can be NULL */
+          samples[index].timestamp = timestamp;
-    stss = qtdemux_tree_get_child_by_type (stbl, FOURCC_stss);
+          /* add non-scaled values to avoid rounding errors */
+          time += duration;
+          timestamp =
+              gst_util_uint64_scale (time, GST_SECOND, stream->timescale);
+          samples[index].duration = timestamp - samples[index].timestamp;
 
-    if (stss) {
+          index++;
-      /* mark keyframes */
+          if (G_UNLIKELY (index >= n_samples))
-      guint32 n_sample_syncs;
+            goto done3;
-      const guint8 *stss_p = (guint8 *) stss->data;
-
-      stss_p += 12;
-      n_sample_syncs = QT_UINT32 (stss_p);
-      if (n_sample_syncs == 0) {
-        stream->all_keyframe = TRUE;
-      } else {
-        for (i = 0; i < n_sample_syncs; i++) {
-          stss_p += 4;
-          /* note that the first sample is index 1, not 0 */
-          index = QT_UINT32 (stss_p);
-          if (G_LIKELY (index > 0 && index <= n_samples))
-            samples[index - 1].keyframe = TRUE;
         }
       }
-      stps = qtdemux_tree_get_child_by_type (stbl, FOURCC_stps);
+      /* fill up empty timestamps with the last timestamp, this can happen when
-      if (stps) {
+       * the last samples do not decode and so we don't have timestamps for them.
-        /* stps marks partial sync frames like open GOP I-Frames */
+       * We however look at the last timestamp to estimate the track length so we
-        guint32 n_sample_syncs;
+       * need something in here. */
-        const guint8 *stps_p = (guint8 *) stps->data;
+      for (; index < n_samples; index++) {
+        GST_DEBUG_OBJECT (qtdemux, "fill sample %d: timestamp %" GST_TIME_FORMAT,
+            index, GST_TIME_ARGS (timestamp));
+        samples[index].timestamp = timestamp;
+        samples[index].duration = -1;
+      }
+    }
+  done3:
+    {
+      /* FIXME: split this block out into a separate function */
+      QtAtomParser stss, stps;
 
-        stps_p += 12;
+      /* sample sync, can be NULL */
-        n_sample_syncs = QT_UINT32 (stps_p);
+      if (qtdemux_tree_get_child_by_type_full (stbl, FOURCC_stss, &stss)) {
-        if (n_sample_syncs != 0) {
+        guint32 n_sample_syncs;
-          /* no entries, the stss table contains the real sync
+
-           * samples */
+        /* mark keyframes */
+        if (!qt_atom_parser_skip (&stss, 4))
+          goto corrupt_file;
+        if (!qt_atom_parser_get_uint32 (&stss, &n_sample_syncs))
+          goto corrupt_file;
+
+        if (n_sample_syncs == 0) {
+          stream->all_keyframe = TRUE;
         } else {
+          /* make sure there's enough data */
+          if (qt_atom_parser_get_remaining (&stss) < (n_sample_syncs * 4))
+            goto corrupt_file;
           for (i = 0; i < n_sample_syncs; i++) {
-            stps_p += 4;
             /* note that the first sample is index 1, not 0 */
-            index = QT_UINT32 (stps_p);
+            index = qt_atom_parser_get_uint32_unchecked (&stss);
             if (G_LIKELY (index > 0 && index <= n_samples))
               samples[index - 1].keyframe = TRUE;
           }
         }
+        /* stps marks partial sync frames like open GOP I-Frames */
+        if (qtdemux_tree_get_child_by_type_full (stbl, FOURCC_stps, &stps)) {
+          guint32 n_sample_syncs;
+
+          if (!qt_atom_parser_skip (&stps, 4))
+            goto corrupt_file;
+          if (!qt_atom_parser_get_uint32 (&stps, &n_sample_syncs))
+            goto corrupt_file;
+
+          if (n_sample_syncs != 0) {
+            /* no entries, the stss table contains the real sync
+             * samples */
+          } else {
+            /* make sure there's enough data */
+            if (qt_atom_parser_get_remaining (&stps) < (n_sample_syncs * 4))
+              goto corrupt_file;
+            for (i = 0; i < n_sample_syncs; i++) {
+              /* note that the first sample is index 1, not 0 */
+              index = qt_atom_parser_get_uint32_unchecked (&stps);
+              if (G_LIKELY (index > 0 && index <= n_samples))
+                samples[index - 1].keyframe = TRUE;
+            }
+          }
+        }
+      } else {
+        /* no stss, all samples are keyframes */
+        stream->all_keyframe = TRUE;
       }
-    } else {
-      /* no stss, all samples are keyframes */
-      stream->all_keyframe = TRUE;
     }
   } else {
     GST_DEBUG_OBJECT (qtdemux,
         "stsz sample_size %d != 0, treating chunks as samples", sample_size);
+
+    /* skip version + flags */
+    if (!gst_byte_reader_skip (&co_reader, 1 + 3))
+      goto corrupt_file;
+
     /* treat chunks as samples */
-    if (stco) {
+    if (!gst_byte_reader_get_uint32_be (&co_reader, &n_samples))
-      n_samples = QT_UINT32 (stco_data + 12);
+      goto corrupt_file;
-    } else {
-      n_samples = QT_UINT32 (co64_data + 12);
-    }
 
     if (n_samples == 0)
       goto no_samples;
@@ -3766,6 +3793,7 @@ qtdemux_parse_samples (GstQTDemux * qtdemux, QtDemuxStream * stream,
       goto corrupt_file;
 
     for (i = 0; i < n_samples_per_chunk; i++) {
+      QtAtomParser co_chunk;
       guint32 first_chunk, last_chunk;
       guint32 samples_per_chunk;
 
@@ -3796,22 +3824,28 @@ qtdemux_parse_samples (GstQTDemux * qtdemux, QtDemuxStream * stream,
           "entry %d has first_chunk %d, last_chunk %d, samples_per_chunk %d", i,
           first_chunk, last_chunk, samples_per_chunk);
 
-      for (j = first_chunk; j < last_chunk; j++) {
+      if (G_UNLIKELY (last_chunk < first_chunk))
-        guint64 chunk_offset;
+        goto corrupt_file;
 
+      if (last_chunk != G_MAXUINT32) {
+        if (!qt_atom_parser_peek_sub (&co_reader, first_chunk * co_size,
+                (last_chunk - first_chunk) * co_size, &co_chunk))
+          goto corrupt_file;
+      } else {
+        co_chunk = co_reader;
+        if (!qt_atom_parser_skip (&co_chunk, first_chunk * co_size))
+          goto corrupt_file;
+      }
+
+      for (j = first_chunk; j < last_chunk; j++) {
         if (j >= n_samples)
           goto done;
 
-        if (stco) {
+        samples[j].offset =
-          chunk_offset = QT_UINT32 (stco_data + 16 + j * 4);
+            qt_atom_parser_get_offset_unchecked (&co_chunk, co_size);
-        } else {
-          chunk_offset = QT_UINT64 (co64_data + 16 + j * 8);
-        }
-        GST_LOG_OBJECT (qtdemux,
-            "Creating entry %d with offset %" G_GUINT64_FORMAT, j,
-            chunk_offset);
 
-        samples[j].offset = chunk_offset;
+        GST_LOG_OBJECT (qtdemux, "Created entry %d with offset "
+            "%" G_GUINT64_FORMAT, j, samples[j].offset);
 
         if (stream->samples_per_frame * stream->bytes_per_frame) {
           samples[j].size = (samples_per_chunk * stream->n_channels) /