oggdemux: Ensure that no pad values are set when setting up the mapper
authorSebastian Dröge <sebastian@centricular.com>
Mon, 27 Aug 2018 08:07:47 +0000 (11:07 +0300)
committerSebastian Dröge <sebastian@centricular.com>
Mon, 27 Aug 2018 08:07:47 +0000 (11:07 +0300)
Otherwise we might have arbitrary values set that are used later and can
cause undefined behaviour, as found by ossfuzz.

ext/ogg/gstoggstream.c

index ff69cc3..cdb6fa6 100644 (file)
@@ -424,6 +424,12 @@ setup_theora_mapper (GstOggStream * pad, ogg_packet * packet)
 
   pad->granulerate_n = GST_READ_UINT32_BE (data + 22);
   pad->granulerate_d = GST_READ_UINT32_BE (data + 26);
+  if (pad->granulerate_n == 0 || pad->granulerate_d == 0) {
+    GST_WARNING ("frame rate %d/%d", pad->granulerate_n, pad->granulerate_d);
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    return FALSE;
+  }
 
   par_n = GST_READ_UINT24_BE (data + 30);
   par_d = GST_READ_UINT24_BE (data + 33);
@@ -437,6 +443,9 @@ setup_theora_mapper (GstOggStream * pad, ogg_packet * packet)
   if (pad->granuleshift >= 63) {
     /* Granuleshift can't be greater than the storage size of a granule */
     GST_WARNING ("Invalid granuleshift (%u >= 63)", pad->granuleshift);
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    pad->granuleshift = -1;
     return FALSE;
   }
   GST_LOG ("granshift: %d", pad->granuleshift);
@@ -448,11 +457,6 @@ setup_theora_mapper (GstOggStream * pad, ogg_packet * packet)
   pad->bitrate = GST_READ_UINT24_BE (data + 37);
   GST_LOG ("bit rate: %d", pad->bitrate);
 
-  if (pad->granulerate_n == 0 || pad->granulerate_d == 0) {
-    GST_WARNING ("frame rate %d/%d", pad->granulerate_n, pad->granulerate_d);
-    return FALSE;
-  }
-
   /* The interpretation of the granule position has changed with 3.2.1.
      The granule is now made from the number of frames encoded, rather than
      the index of the frame being encoded - so there is a difference of 1. */
@@ -556,6 +560,11 @@ setup_dirac_mapper (GstOggStream * pad, ogg_packet * packet)
     return FALSE;
   }
 
+  if (header.interlaced_coding != 0) {
+    GST_DEBUG ("non-progressive Dirac coding not implemented");
+    return FALSE;
+  }
+
   pad->is_video = TRUE;
   pad->always_flush_page = TRUE;
   pad->granulerate_n = header.frame_rate_numerator * 2;
@@ -564,11 +573,6 @@ setup_dirac_mapper (GstOggStream * pad, ogg_packet * packet)
   pad->n_header_packets = 1;
   pad->frame_size = 2;
 
-  if (header.interlaced_coding != 0) {
-    GST_DEBUG ("non-progressive Dirac coding not implemented");
-    return FALSE;
-  }
-
   pad->caps = gst_caps_new_simple ("video/x-dirac",
       "width", G_TYPE_INT, header.width,
       "height", G_TYPE_INT, header.height,
@@ -861,8 +865,15 @@ setup_vorbis_mapper (GstOggStream * pad, ogg_packet * packet)
   data += 4;
   chans = GST_READ_UINT8 (data);
   data += 1;
+
   pad->granulerate_n = GST_READ_UINT32_LE (data);
   pad->granulerate_d = 1;
+  if (pad->granulerate_n == 0) {
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    return FALSE;
+  }
+
   pad->granuleshift = 0;
   pad->preroll = 2;
   pad->last_size = 0;
@@ -888,9 +899,6 @@ setup_vorbis_mapper (GstOggStream * pad, ogg_packet * packet)
 
   pad->n_header_packets = 3;
 
-  if (pad->granulerate_n == 0)
-    return FALSE;
-
   gst_parse_vorbis_header_packet (pad, packet);
 
   pad->caps = gst_caps_new_simple ("audio/x-vorbis",
@@ -983,8 +991,15 @@ setup_speex_mapper (GstOggStream * pad, ogg_packet * packet)
   guint chans;
 
   data += 8 + 20 + 4 + 4;
+
   pad->granulerate_n = GST_READ_UINT32_LE (data);
   pad->granulerate_d = 1;
+  if (pad->granulerate_n == 0) {
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    return FALSE;
+  }
+
   pad->granuleshift = 0;
 
   data += 4 + 4 + 4;
@@ -999,9 +1014,6 @@ setup_speex_mapper (GstOggStream * pad, ogg_packet * packet)
   pad->frame_size = GST_READ_UINT32_LE (packet->packet + 64) *
       GST_READ_UINT32_LE (packet->packet + 56);
 
-  if (pad->granulerate_n == 0)
-    return FALSE;
-
   pad->caps = gst_caps_new_simple ("audio/x-speex", "rate", G_TYPE_INT,
       pad->granulerate_n, "channels", G_TYPE_INT, chans, NULL);
 
@@ -1079,6 +1091,13 @@ setup_flac_mapper (GstOggStream * pad, ogg_packet * packet)
 
   pad->granulerate_n = (GST_READ_UINT32_BE (data + 27) & 0xFFFFF000) >> 12;
   pad->granulerate_d = 1;
+
+  if (pad->granulerate_n == 0) {
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    return FALSE;
+  }
+
   pad->granuleshift = 0;
   chans = ((GST_READ_UINT32_BE (data + 27) & 0x00000E00) >> 9) + 1;
 
@@ -1086,9 +1105,6 @@ setup_flac_mapper (GstOggStream * pad, ogg_packet * packet)
 
   pad->n_header_packets = GST_READ_UINT16_BE (packet->packet + 7);
 
-  if (pad->granulerate_n == 0)
-    return FALSE;
-
   pad->caps = gst_caps_new_simple ("audio/x-flac", "rate", G_TYPE_INT,
       pad->granulerate_n, "channels", G_TYPE_INT, chans, NULL);
 
@@ -1325,8 +1341,6 @@ gst_ogg_map_add_fisbone (GstOggStream * pad, GstOggStream * skel_pad,
   /* skip "fisbone\0" + headers offset + serialno + num headers */
   data += 8 + 4 + 4 + 4;
 
-  pad->have_fisbone = TRUE;
-
   /* We don't overwrite whatever was set before by the format-specific
      setup: skeleton contains wrong information sometimes, and the codec
      headers are authoritative.
@@ -1343,10 +1357,15 @@ gst_ogg_map_add_fisbone (GstOggStream * pad, GstOggStream * skel_pad,
     if (pad->granuleshift >= 63) {
       /* Granuleshift can't be greater than the storage size of a granule */
       GST_WARNING ("Invalid granuleshift (%u >= 63)", pad->granuleshift);
+      pad->granulerate_n = 0;
+      pad->granulerate_d = 0;
+      pad->granuleshift = -1;
       return FALSE;
     }
   }
 
+  pad->have_fisbone = TRUE;
+
   start_granule = GST_READ_UINT64_LE (data + 16);
   pad->preroll = GST_READ_UINT32_LE (data + 24);
 
@@ -1618,6 +1637,13 @@ setup_ogmaudio_mapper (GstOggStream * pad, ogg_packet * packet)
   pad->granulerate_n = GST_READ_UINT64_LE (data + 25);
   pad->granulerate_d = 1;
 
+  GST_LOG ("sample rate: %d", pad->granulerate_n);
+  if (pad->granulerate_n == 0) {
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    return FALSE;
+  }
+
   fourcc = GST_READ_UINT32_LE (data + 9);
   fstr = g_strdup_printf ("%" GST_FOURCC_FORMAT, GST_FOURCC_ARGS (fourcc));
   GST_DEBUG ("fourcc: %s", fstr);
@@ -1626,10 +1652,6 @@ setup_ogmaudio_mapper (GstOggStream * pad, ogg_packet * packet)
   pad->caps =
       gst_riff_create_audio_caps (fourcc, NULL, NULL, NULL, NULL, NULL, NULL);
 
-  GST_LOG ("sample rate: %d", pad->granulerate_n);
-  if (pad->granulerate_n == 0)
-    return FALSE;
-
   if (pad->caps) {
     gst_caps_set_simple (pad->caps,
         "rate", G_TYPE_INT, pad->granulerate_n, NULL);
@@ -1716,8 +1738,11 @@ setup_ogmtext_mapper (GstOggStream * pad, ogg_packet * packet)
       pad->granulerate_n, pad->granulerate_d,
       (double) pad->granulerate_n / pad->granulerate_d);
 
-  if (pad->granulerate_d <= 0)
+  if (pad->granulerate_d <= 0) {
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
     return FALSE;
+  }
 
   pad->caps = gst_caps_new_simple ("text/x-raw", "format", G_TYPE_STRING,
       "utf8", NULL);
@@ -1762,14 +1787,14 @@ setup_pcm_mapper (GstOggStream * pad, ogg_packet * packet)
   pad->granulerate_d = 1;
   GST_LOG ("sample rate: %d", pad->granulerate_n);
 
-  format = GST_READ_UINT32_LE (data + 12);
-  channels = GST_READ_UINT8 (data + 21);
-
-  pad->n_header_packets = 2 + GST_READ_UINT32_LE (data + 24);
-
-  if (pad->granulerate_n == 0)
+  if (pad->granulerate_n == 0) {
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
     return FALSE;
+  }
 
+  format = GST_READ_UINT32_LE (data + 12);
+  channels = GST_READ_UINT8 (data + 21);
   switch (format) {
     case OGGPCM_FMT_S8:
       caps = gst_caps_new_simple ("audio/x-raw",
@@ -1826,9 +1851,13 @@ setup_pcm_mapper (GstOggStream * pad, ogg_packet * packet)
           "format", G_TYPE_STRING, "F64BE", NULL);
       break;
     default:
+      pad->granulerate_n = 0;
+      pad->granulerate_d = 0;
       return FALSE;
   }
 
+  pad->n_header_packets = 2 + GST_READ_UINT32_LE (data + 24);
+
   gst_caps_set_simple (caps,
       "layout", G_TYPE_STRING, "interleaved",
       "rate", G_TYPE_INT, pad->granulerate_n,
@@ -1848,17 +1877,25 @@ setup_cmml_mapper (GstOggStream * pad, ogg_packet * packet)
   pad->granulerate_n = GST_READ_UINT64_LE (data + 12);
   pad->granulerate_d = GST_READ_UINT64_LE (data + 20);
   pad->granuleshift = data[28];
+
   if (pad->granuleshift >= 63) {
     /* Granuleshift can't be greater than the storage size of a granule */
     GST_WARNING ("Invalid granuleshift (%u >= 63)", pad->granuleshift);
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    pad->granuleshift = -1;
     return FALSE;
   }
   GST_LOG ("sample rate: %d", pad->granulerate_n);
 
-  pad->n_header_packets = 3;
-
-  if (pad->granulerate_n == 0)
+  if (pad->granulerate_n == 0) {
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    pad->granuleshift = -1;
     return FALSE;
+  }
+
+  pad->n_header_packets = 3;
 
   data += 4 + (4 + 4 + 4);
   GST_DEBUG ("blocksize0: %u", 1 << (data[0] >> 4));
@@ -1887,8 +1924,12 @@ setup_celt_mapper (GstOggStream * pad, ogg_packet * packet)
   pad->frame_size = GST_READ_UINT32_LE (packet->packet + 44);
   pad->n_header_packets = GST_READ_UINT32_LE (packet->packet + 56) + 2;
 
-  if (pad->granulerate_n == 0)
+  if (pad->granulerate_n == 0) {
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    pad->granuleshift = -1;
     return FALSE;
+  }
 
   pad->caps = gst_caps_new_simple ("audio/x-celt",
       "rate", G_TYPE_INT, pad->granulerate_n, NULL);
@@ -1913,16 +1954,23 @@ setup_kate_mapper (GstOggStream * pad, ogg_packet * packet)
   if (pad->granuleshift >= 63) {
     /* Granuleshift can't be greater than the storage size of a granule */
     GST_WARNING ("Invalid granuleshift (%u >= 63)", pad->granuleshift);
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    pad->granuleshift = -1;
     return FALSE;
   }
   GST_LOG ("sample rate: %d", pad->granulerate_n);
 
+  if (pad->granulerate_n == 0) {
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    pad->granuleshift = -1;
+    return FALSE;
+  }
+
   pad->n_header_packets = GST_READ_UINT8 (data + 11);
   GST_LOG ("kate header packets: %d", pad->n_header_packets);
 
-  if (pad->granulerate_n == 0)
-    return FALSE;
-
   category = (const char *) data + 48;
   if (strcmp (category, "subtitles") == 0 || strcmp (category, "SUB") == 0 ||
       strcmp (category, "spu-subtitles") == 0 ||
@@ -2178,19 +2226,25 @@ setup_daala_mapper (GstOggStream * pad, ogg_packet * packet)
   if (pad->granuleshift >= 63) {
     /* Granuleshift can't be greater than the storage size of a granule */
     GST_WARNING ("Invalid granuleshift (%u >= 63)", pad->granuleshift);
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    pad->granuleshift = -1;
     return FALSE;
   }
   GST_LOG ("granshift: %d", pad->granuleshift);
 
-  pad->is_video = TRUE;
-  pad->n_header_packets = 3;
-  pad->frame_size = 1;
-
   if (pad->granulerate_n == 0 || pad->granulerate_d == 0) {
     GST_WARNING ("frame rate %d/%d", pad->granulerate_n, pad->granulerate_d);
+    pad->granulerate_n = 0;
+    pad->granulerate_d = 0;
+    pad->granuleshift = -1;
     return FALSE;
   }
 
+  pad->is_video = TRUE;
+  pad->n_header_packets = 3;
+  pad->frame_size = 1;
+
   pad->caps = gst_caps_new_empty_simple ("video/x-daala");
 
   if (w > 0 && h > 0) {