wavparse: push mode; fix/improve chunk handling

Handle large, invalid or otherwise unusual chunk sizes.
Verify some chunk sizes to be at least the size they are
expected to be and round up some sizes to even number for
e.g. offset administration, which must also be properly
tracked in push mode.

2 files changed, 81 insertions, 27 deletions

diff --git a/gst/wavparse/gstwavparse.c b/gst/wavparse/gstwavparse.c
index abcbee12..7beb7327 100644
--- a/gst/wavparse/gstwavparse.c
+++ b/gst/wavparse/gstwavparse.c
@@ -1075,10 +1075,20 @@ gst_wavparse_peek_chunk (GstWavParse * wav, guint32 * tag, guint32 * size)
   if (!gst_wavparse_peek_chunk_info (wav, tag, size))
     return FALSE;
 
-  GST_DEBUG ("Need to peek chunk of %d bytes", *size);
+  /* size 0 -> empty data buffer would surprise most callers,
+   * large size -> do not bother trying to squeeze that into adapter,
+   * so we throw poor man's exception, which can be caught if caller really
+   * wants to handle 0 size chunk */
+  if (!(*size) || (*size) >= (1 << 30)) {
+    GST_INFO ("Invalid/unexpected chunk size %d for tag %" GST_FOURCC_FORMAT,
+        *size, GST_FOURCC_ARGS (*tag));
+    /* chain should give up */
+    wav->abort_buffering = TRUE;
+    return FALSE;
+  }
   peek_size = (*size + 1) & ~1;
-
   available = gst_adapter_available (wav->adapter);
+
   if (available >= (8 + peek_size)) {
     return TRUE;
   } else {
@@ -1118,7 +1128,7 @@ gst_wavparse_calculate_duration (GstWavParse * wav)
   return FALSE;
 }
 
-static void
+static gboolean
 gst_waveparse_ignore_chunk (GstWavParse * wav, GstBuffer * buf, guint32 tag,
     guint32 size)
 {
@@ -1126,7 +1136,7 @@ gst_waveparse_ignore_chunk (GstWavParse * wav, GstBuffer * buf, guint32 tag,
 
   if (wav->streaming) {
     if (!gst_wavparse_peek_chunk (wav, &tag, &size))
-      return;
+      return FALSE;
   }
   GST_DEBUG_OBJECT (wav, "Ignoring tag %" GST_FOURCC_FORMAT,
       GST_FOURCC_ARGS (tag));
@@ -1137,6 +1147,8 @@ gst_waveparse_ignore_chunk (GstWavParse * wav, GstBuffer * buf, guint32 tag,
   } else {
     gst_buffer_unref (buf);
   }
+
+  return TRUE;
 }
 
 static GstFlowReturn
@@ -1165,7 +1177,14 @@ gst_wavparse_stream_headers (GstWavParse * wav)
       gst_adapter_flush (wav->adapter, 8);
       wav->offset += 8;
 
-      buf = gst_adapter_take_buffer (wav->adapter, size);
+      if (size) {
+        buf = gst_adapter_take_buffer (wav->adapter, size);
+        if (size & 1)
+          gst_adapter_flush (wav->adapter, 1);
+        wav->offset += GST_ROUND_UP_2 (size);
+      } else {
+        buf = gst_buffer_new ();
+      }
     } else {
       if ((res = gst_riff_read_chunk (GST_ELEMENT_CAST (wav), wav->sinkpad,
                   &wav->offset, &tag, &buf)) != GST_FLOW_OK)
@@ -1342,17 +1361,27 @@ gst_wavparse_stream_headers (GstWavParse * wav)
             wav->format != GST_RIFF_WAVE_FORMAT_MPEGL3) {
           const guint data_size = 4;
 
+          GST_INFO_OBJECT (wav, "Have fact chunk");
+          if (size < data_size) {
+            if (!gst_waveparse_ignore_chunk (wav, buf, tag, size)) {
+              /* need more data */
+              return GST_FLOW_OK;
+            }
+            GST_DEBUG_OBJECT (wav, "need %d, available %d; ignoring chunk",
+                data_size, size);
+            break;
+          }
           /* number of samples (for compressed formats) */
           if (wav->streaming) {
             const guint8 *data = NULL;
 
-            if (gst_adapter_available (wav->adapter) < 8 + data_size) {
+            if (!gst_wavparse_peek_chunk (wav, &tag, &size)) {
               return GST_FLOW_OK;
             }
             gst_adapter_flush (wav->adapter, 8);
             data = gst_adapter_peek (wav->adapter, data_size);
             wav->fact = GST_READ_UINT32_LE (data);
-            gst_adapter_flush (wav->adapter, data_size);
+            gst_adapter_flush (wav->adapter, GST_ROUND_UP_2 (size));
           } else {
             gst_buffer_unref (buf);
             if ((res =
@@ -1363,10 +1392,13 @@ gst_wavparse_stream_headers (GstWavParse * wav)
             gst_buffer_unref (buf);
           }
           GST_DEBUG_OBJECT (wav, "have fact %u", wav->fact);
-          wav->offset += 8 + data_size;
+          wav->offset += 8 + GST_ROUND_UP_2 (size);
           break;
         } else {
-          gst_waveparse_ignore_chunk (wav, buf, tag, size);
+          if (!gst_waveparse_ignore_chunk (wav, buf, tag, size)) {
+            /* need more data */
+            return GST_FLOW_OK;
+          }
         }
         break;
       }
@@ -1374,8 +1406,18 @@ gst_wavparse_stream_headers (GstWavParse * wav)
         const gst_riff_acid *acid = NULL;
         const guint data_size = sizeof (gst_riff_acid);
 
+        GST_INFO_OBJECT (wav, "Have acid chunk");
+        if (size < data_size) {
+          if (!gst_waveparse_ignore_chunk (wav, buf, tag, size)) {
+            /* need more data */
+            return GST_FLOW_OK;
+          }
+          GST_DEBUG_OBJECT (wav, "need %d, available %d; ignoring chunk",
+              data_size, size);
+          break;
+        }
         if (wav->streaming) {
-          if (gst_adapter_available (wav->adapter) < 8 + data_size) {
+          if (!gst_wavparse_peek_chunk (wav, &tag, &size)) {
             return GST_FLOW_OK;
           }
           gst_adapter_flush (wav->adapter, 8);
@@ -1385,23 +1427,23 @@ gst_wavparse_stream_headers (GstWavParse * wav)
           gst_buffer_unref (buf);
           if ((res =
                   gst_pad_pull_range (wav->sinkpad, wav->offset + 8,
-                      data_size, &buf)) != GST_FLOW_OK)
+                      size, &buf)) != GST_FLOW_OK)
             goto header_read_error;
           acid = (const gst_riff_acid *) GST_BUFFER_DATA (buf);
         }
-        GST_INFO_OBJECT (wav, "Have acid chunk");
         /* send data as tags */
         if (!wav->tags)
           wav->tags = gst_tag_list_new ();
         gst_tag_list_add (wav->tags, GST_TAG_MERGE_REPLACE,
             GST_TAG_BEATS_PER_MINUTE, acid->tempo, NULL);
 
+        size = GST_ROUND_UP_2 (size);
         if (wav->streaming) {
-          gst_adapter_flush (wav->adapter, data_size);
+          gst_adapter_flush (wav->adapter, size);
         } else {
           gst_buffer_unref (buf);
-          wav->offset += 8 + data_size;
         }
+        wav->offset += 8 + size;
         break;
       }
         /* FIXME: all list tags after data are ignored in streaming mode */
@@ -1426,18 +1468,21 @@ gst_wavparse_stream_headers (GstWavParse * wav)
         }
         switch (ltag) {
           case GST_RIFF_LIST_INFO:{
-            const guint data_size = size - 4;
+            const gint data_size = size - 4;
             GstTagList *new;
 
             GST_INFO_OBJECT (wav, "Have LIST chunk INFO size %u", data_size);
             if (wav->streaming) {
-              gst_adapter_flush (wav->adapter, 12);
-              if (gst_adapter_available (wav->adapter) < data_size) {
+              if (!gst_wavparse_peek_chunk (wav, &tag, &size)) {
                 return GST_FLOW_OK;
               }
-              gst_buffer_unref (buf);
-              if (data_size > 0)
+              gst_adapter_flush (wav->adapter, 12);
+              wav->offset += 12;
+              if (data_size > 0) {
                 buf = gst_adapter_take_buffer (wav->adapter, data_size);
+                if (data_size & 1)
+                  gst_adapter_flush (wav->adapter, 1);
+              }
             } else {
               wav->offset += 12;
               gst_buffer_unref (buf);
@@ -1459,25 +1504,26 @@ gst_wavparse_stream_headers (GstWavParse * wav)
                   gst_tag_list_free (old);
                 gst_tag_list_free (new);
               }
-              if (wav->streaming) {
-                gst_adapter_flush (wav->adapter, data_size);
-              } else {
-                gst_buffer_unref (buf);
-                wav->offset += data_size;
-              }
+              gst_buffer_unref (buf);
+              wav->offset += GST_ROUND_UP_2 (data_size);
             }
             break;
           }
           default:
             GST_INFO_OBJECT (wav, "Ignoring LIST chunk %" GST_FOURCC_FORMAT,
                 GST_FOURCC_ARGS (ltag));
-            gst_waveparse_ignore_chunk (wav, buf, tag, size);
+            if (!gst_waveparse_ignore_chunk (wav, buf, tag, size))
+              /* need more data */
+              return GST_FLOW_OK;
             break;
         }
         break;
       }
       default:
-        gst_waveparse_ignore_chunk (wav, buf, tag, size);
+        if (!gst_waveparse_ignore_chunk (wav, buf, tag, size))
+          /* need more data */
+          return GST_FLOW_OK;
+        break;
     }
 
     if (upstream_size && (wav->offset >= upstream_size)) {
@@ -2030,6 +2076,13 @@ gst_wavparse_chain (GstPad * pad, GstBuffer * buf)
       g_return_val_if_reached (GST_FLOW_ERROR);
   }
 done:
+  if (G_UNLIKELY (wav->abort_buffering)) {
+    wav->abort_buffering = FALSE;
+    ret = GST_FLOW_ERROR;
+    /* sort of demux/parse error */
+    GST_ELEMENT_ERROR (wav, STREAM, DEMUX, NULL, ("unhandled buffer size"));
+  }
+
   return ret;
 }
 
diff --git a/gst/wavparse/gstwavparse.h b/gst/wavparse/gstwavparse.h
index 97cf27f8..29ce814b 100644
--- a/gst/wavparse/gstwavparse.h
+++ b/gst/wavparse/gstwavparse.h
@@ -71,6 +71,7 @@ struct _GstWavParse {
 
   /* WAVE decoding state */
   GstWavParseState state;
+  gboolean abort_buffering;
 
   /* format of audio, see defines below */
   gint format;