From 253831a5499ca02dee165cff337c2061fd034f07 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Tue, 22 Aug 2006 16:31:47 +0000 Subject: [PATCH] gst-libs/gst/riff/riff-read.c: Protect public functions against bad input. Original commit message from CVS: * gst-libs/gst/riff/riff-read.c: (gst_riff_read_chunk), (gst_riff_parse_chunk), (gst_riff_parse_file_header), (gst_riff_parse_strh), (gst_riff_parse_strf_vids), (gst_riff_parse_strf_auds), (gst_riff_parse_strf_iavs), (gst_riff_parse_info): Protect public functions against bad input. Do some cleanups. Fix documentation. --- ChangeLog | 11 ++ gst-libs/gst/riff/riff-read.c | 262 +++++++++++++++++++++------------- 2 files changed, 176 insertions(+), 97 deletions(-) diff --git a/ChangeLog b/ChangeLog index da91b32485..22511b0bf8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2006-08-22 Wim Taymans + + * gst-libs/gst/riff/riff-read.c: (gst_riff_read_chunk), + (gst_riff_parse_chunk), (gst_riff_parse_file_header), + (gst_riff_parse_strh), (gst_riff_parse_strf_vids), + (gst_riff_parse_strf_auds), (gst_riff_parse_strf_iavs), + (gst_riff_parse_info): + Protect public functions against bad input. + Do some cleanups. + Fix documentation. + 2006-08-22 Tim-Philipp Müller * gst-libs/gst/riff/riff-ids.h: diff --git a/gst-libs/gst/riff/riff-read.c b/gst-libs/gst/riff/riff-read.c index 6930d43e07..143415b385 100644 --- a/gst-libs/gst/riff/riff-read.c +++ b/gst-libs/gst/riff/riff-read.c @@ -54,15 +54,18 @@ gst_riff_read_chunk (GstElement * element, guint size; guint64 offset = *_offset; -skip_junk: + g_return_val_if_fail (element != NULL, GST_FLOW_ERROR); + g_return_val_if_fail (pad != NULL, GST_FLOW_ERROR); + g_return_val_if_fail (_offset != NULL, GST_FLOW_ERROR); + g_return_val_if_fail (tag != NULL, GST_FLOW_ERROR); + g_return_val_if_fail (_chunk_data != NULL, GST_FLOW_ERROR); - if ((res = gst_pad_pull_range (pad, offset, 8, &buf)) != GST_FLOW_OK) +skip_junk: + size = 8; + if ((res = gst_pad_pull_range (pad, offset, size, &buf)) != GST_FLOW_OK) return res; - else if (!buf || GST_BUFFER_SIZE (buf) < 8) { - if (buf) - gst_buffer_unref (buf); - return GST_FLOW_ERROR; - } + else if (GST_BUFFER_SIZE (buf) < size) + goto too_small; *tag = GST_READ_UINT32_LE (GST_BUFFER_DATA (buf)); size = GST_READ_UINT32_LE (GST_BUFFER_DATA (buf) + 4); @@ -73,26 +76,32 @@ skip_junk: /* skip 'JUNK' chunks */ if (*tag == GST_RIFF_TAG_JUNK) { - *_offset += 8 + GST_ROUND_UP_2 (size); - offset += 8 + GST_ROUND_UP_2 (size); + size = GST_ROUND_UP_2 (size); + *_offset += 8 + size; + offset += 8 + size; GST_DEBUG_OBJECT (element, "skipping JUNK chunk"); goto skip_junk; } if ((res = gst_pad_pull_range (pad, offset + 8, size, &buf)) != GST_FLOW_OK) return res; - else if (!buf || GST_BUFFER_SIZE (buf) < size) { - GST_DEBUG_OBJECT (element, "not enough data (available=%u, needed=%u)", - (buf) ? GST_BUFFER_SIZE (buf) : 0, size); - if (buf) - gst_buffer_unref (buf); - return GST_FLOW_UNEXPECTED; - } + else if (GST_BUFFER_SIZE (buf) < size) + goto too_small; *_chunk_data = buf; *_offset += 8 + ((size + 1) & ~1); return GST_FLOW_OK; + + /* ERRORS */ +too_small: + { + /* short read, we return UNEXPECTED to mark the EOS case */ + GST_DEBUG_OBJECT (element, "not enough data (available=%u, needed=%u)", + GST_BUFFER_SIZE (buf), size); + gst_buffer_unref (buf); + return GST_FLOW_UNEXPECTED; + } } /** @@ -109,55 +118,70 @@ skip_junk: * * Returns: the fourcc tag of this chunk, or FALSE on error */ - gboolean gst_riff_parse_chunk (GstElement * element, GstBuffer * buf, guint * _offset, guint32 * _fourcc, GstBuffer ** chunk_data) { - guint size; + guint size, bufsize; guint32 fourcc; guint8 *data; - gchar dbg[5] = { 0, }; guint offset = *_offset; + g_return_val_if_fail (element != NULL, FALSE); + g_return_val_if_fail (buf != NULL, FALSE); + g_return_val_if_fail (_offset != NULL, FALSE); + g_return_val_if_fail (_fourcc != NULL, FALSE); + g_return_val_if_fail (chunk_data != NULL, FALSE); + *chunk_data = NULL; *_fourcc = 0; - if (buf && GST_BUFFER_SIZE (buf) == offset) { - GST_DEBUG_OBJECT (element, "End of chunk (offset %d)", offset); - return FALSE; - } + bufsize = GST_BUFFER_SIZE (buf); - if (!buf || GST_BUFFER_SIZE (buf) < offset + 8) { - GST_DEBUG_OBJECT (element, - "Failed to parse chunk header (offset %d, %d available, %d needed)", - offset, (buf) ? GST_BUFFER_SIZE (buf) : 0, 8); - return FALSE; - } + if (bufsize == offset) + goto end_offset; + + if (bufsize < offset + 8) + goto too_small; /* read header */ data = GST_BUFFER_DATA (buf) + offset; fourcc = GST_READ_UINT32_LE (data); size = GST_READ_UINT32_LE (data + 4); - memcpy (dbg, data, 4); - GST_DEBUG_OBJECT (element, "fourcc=%s, size=%u", dbg, size); + GST_DEBUG_OBJECT (element, "fourcc=%" GST_FOURCC_FORMAT ", size=%u", + GST_FOURCC_ARGS (fourcc), size); - if (GST_BUFFER_SIZE (buf) < size + 8 + offset) { + if (bufsize < size + 8 + offset) { GST_DEBUG_OBJECT (element, "Needed chunk data (%d) is more than available (%d), shortcutting", - size, GST_BUFFER_SIZE (buf) - 8 - offset); - size = GST_BUFFER_SIZE (buf) - 8 - offset; + size, bufsize - 8 - offset); + size = bufsize - 8 - offset; } if (size) *chunk_data = gst_buffer_create_sub (buf, offset + 8, size); else *chunk_data = NULL; + *_fourcc = fourcc; *_offset += 8 + ((size + 1) & ~1); return TRUE; + + /* ERRORS */ +end_offset: + { + GST_DEBUG_OBJECT (element, "End of chunk (offset %d)", offset); + return FALSE; + } +too_small: + { + GST_DEBUG_OBJECT (element, + "Failed to parse chunk header (offset %d, %d available, %d needed)", + offset, bufsize, 8); + return FALSE; + } } /** @@ -170,43 +194,54 @@ gst_riff_parse_chunk (GstElement * element, GstBuffer * buf, * * Reads the first few bytes from the provided buffer, checks * if this stream is a RIFF stream, and determines document type. - * The input data is discarded after use. + * This function takes ownership of @buf so it should not be used anymore + * after calling this function. * * Returns: FALSE if this is not a RIFF stream (in which case the * caller should error out; we already throw an error), or TRUE * if it is. */ - gboolean gst_riff_parse_file_header (GstElement * element, GstBuffer * buf, guint32 * doctype) { - guint8 *data = GST_BUFFER_DATA (buf); + guint8 *data; guint32 tag; - if (!buf || GST_BUFFER_SIZE (buf) < 12) { + g_return_val_if_fail (buf != NULL, FALSE); + g_return_val_if_fail (doctype != NULL, FALSE); + + if (GST_BUFFER_SIZE (buf) < 12) + goto too_small; + + data = GST_BUFFER_DATA (buf); + tag = GST_READ_UINT32_LE (data); + if (tag != GST_RIFF_TAG_RIFF) + goto not_riff; + + *doctype = GST_READ_UINT32_LE (data + 8); + + gst_buffer_unref (buf); + + return TRUE; + + /* ERRORS */ +too_small: + { GST_ELEMENT_ERROR (element, STREAM, WRONG_TYPE, (NULL), ("Not enough data to parse RIFF header (%d available, %d needed)", - buf ? GST_BUFFER_SIZE (buf) : 0, 12)); - if (buf) - gst_buffer_unref (buf); + GST_BUFFER_SIZE (buf), 12)); + gst_buffer_unref (buf); return FALSE; } - - tag = GST_READ_UINT32_LE (data); - if (tag != GST_RIFF_TAG_RIFF) { +not_riff: + { GST_ELEMENT_ERROR (element, STREAM, WRONG_TYPE, (NULL), ("Stream is no RIFF stream: %" GST_FOURCC_FORMAT, GST_FOURCC_ARGS (tag))); gst_buffer_unref (buf); return FALSE; } - - *doctype = GST_READ_UINT32_LE (data + 8); - - gst_buffer_unref (buf); - - return TRUE; } /** @@ -216,8 +251,7 @@ gst_riff_parse_file_header (GstElement * element, * @strh: a pointer (returned by this function) to a filled-in * strh structure. Caller should free it. * - * Parses a strh structure from input data. The input data is - * discarded after use. + * Parses a strh structure from input data. Takes ownership of @buf. * * Returns: TRUE if parsing succeeded, otherwise FALSE. The stream * should be skipped on error, but it is not fatal. @@ -229,14 +263,11 @@ gst_riff_parse_strh (GstElement * element, { gst_riff_strh *strh; - if (!buf || GST_BUFFER_SIZE (buf) < sizeof (gst_riff_strh)) { - GST_ERROR_OBJECT (element, - "Too small strh (%d available, %d needed)", - buf ? GST_BUFFER_SIZE (buf) : 0, (int) sizeof (gst_riff_strh)); - if (buf) - gst_buffer_unref (buf); - return FALSE; - } + g_return_val_if_fail (buf != NULL, FALSE); + g_return_val_if_fail (_strh != NULL, FALSE); + + if (GST_BUFFER_SIZE (buf) < sizeof (gst_riff_strh)) + goto too_small; strh = g_memdup (GST_BUFFER_DATA (buf), GST_BUFFER_SIZE (buf)); gst_buffer_unref (buf); @@ -282,6 +313,16 @@ gst_riff_parse_strh (GstElement * element, *_strh = strh; return TRUE; + + /* ERRORS */ +too_small: + { + GST_ERROR_OBJECT (element, + "Too small strh (%d available, %d needed)", + GST_BUFFER_SIZE (buf), (int) sizeof (gst_riff_strh)); + gst_buffer_unref (buf); + return FALSE; + } } /** @@ -295,8 +336,7 @@ gst_riff_parse_strh (GstElement * element, * palette, codec initialization data). * * Parses a video stream´s strf structure plus optionally some - * extradata from input data. The input data is discarded after - * use. + * extradata from input data. This function takes ownership of @buf. * * Returns: TRUE if parsing succeeded, otherwise FALSE. The stream * should be skipped on error, but it is not fatal. @@ -308,14 +348,12 @@ gst_riff_parse_strf_vids (GstElement * element, { gst_riff_strf_vids *strf; - if (!buf || GST_BUFFER_SIZE (buf) < sizeof (gst_riff_strf_vids)) { - GST_ERROR_OBJECT (element, - "Too small strf_vids (%d available, %d needed)", - buf ? GST_BUFFER_SIZE (buf) : 0, (int) sizeof (gst_riff_strf_vids)); - if (buf) - gst_buffer_unref (buf); - return FALSE; - } + g_return_val_if_fail (buf != NULL, FALSE); + g_return_val_if_fail (_strf != NULL, FALSE); + g_return_val_if_fail (data != NULL, FALSE); + + if (GST_BUFFER_SIZE (buf) < sizeof (gst_riff_strf_vids)) + goto too_small; strf = g_memdup (GST_BUFFER_DATA (buf), GST_BUFFER_SIZE (buf)); @@ -368,6 +406,16 @@ gst_riff_parse_strf_vids (GstElement * element, *_strf = strf; return TRUE; + + /* ERRORS */ +too_small: + { + GST_ERROR_OBJECT (element, + "Too small strf_vids (%d available, %d needed)", + GST_BUFFER_SIZE (buf), (int) sizeof (gst_riff_strf_vids)); + gst_buffer_unref (buf); + return FALSE; + } } /** @@ -381,29 +429,29 @@ gst_riff_parse_strf_vids (GstElement * element, * codec initialization data). * * Parses an audio stream´s strf structure plus optionally some - * extradata from input data. The input data is discarded after + * extradata from input data. This function takes ownership of @buf. * use. * * Returns: TRUE if parsing succeeded, otherwise FALSE. The stream * should be skipped on error, but it is not fatal. */ - gboolean gst_riff_parse_strf_auds (GstElement * element, GstBuffer * buf, gst_riff_strf_auds ** _strf, GstBuffer ** data) { gst_riff_strf_auds *strf; + guint bufsize; - if (!buf || GST_BUFFER_SIZE (buf) < sizeof (gst_riff_strf_auds)) { - GST_ERROR_OBJECT (element, - "Too small strf_auds (%d available, %d needed)", - buf ? GST_BUFFER_SIZE (buf) : 0, (int) sizeof (gst_riff_strf_auds)); - if (buf) - gst_buffer_unref (buf); - return FALSE; - } + g_return_val_if_fail (buf != NULL, FALSE); + g_return_val_if_fail (_strf != NULL, FALSE); + g_return_val_if_fail (data != NULL, FALSE); - strf = g_memdup (GST_BUFFER_DATA (buf), GST_BUFFER_SIZE (buf)); + bufsize = GST_BUFFER_SIZE (buf); + + if (bufsize < sizeof (gst_riff_strf_auds)) + goto too_small; + + strf = g_memdup (GST_BUFFER_DATA (buf), bufsize); #if (G_BYTE_ORDER == G_BIG_ENDIAN) strf->format = GUINT16_FROM_LE (strf->format); @@ -416,15 +464,15 @@ gst_riff_parse_strf_auds (GstElement * element, /* size checking */ *data = NULL; - if (GST_BUFFER_SIZE (buf) > sizeof (gst_riff_strf_auds) + 2) { + if (bufsize > sizeof (gst_riff_strf_auds) + 2) { gint len; len = GST_READ_UINT16_LE (&GST_BUFFER_DATA (buf)[16]); - if (len + 2 + sizeof (gst_riff_strf_auds) > GST_BUFFER_SIZE (buf)) { + if (len + 2 + sizeof (gst_riff_strf_auds) > bufsize) { GST_WARNING_OBJECT (element, "Extradata indicated %d bytes, but only %d available", - len, GST_BUFFER_SIZE (buf) - 2 - sizeof (gst_riff_strf_auds)); - len = GST_BUFFER_SIZE (buf) - 2 - sizeof (gst_riff_strf_auds); + len, bufsize - 2 - sizeof (gst_riff_strf_auds)); + len = bufsize - 2 - sizeof (gst_riff_strf_auds); } if (len) *data = gst_buffer_create_sub (buf, sizeof (gst_riff_strf_auds) + 2, len); @@ -446,6 +494,16 @@ gst_riff_parse_strf_auds (GstElement * element, *_strf = strf; return TRUE; + + /* ERROR */ +too_small: + { + GST_ERROR_OBJECT (element, + "Too small strf_auds (%d available, %d needed)", + bufsize, sizeof (gst_riff_strf_auds)); + gst_buffer_unref (buf); + return FALSE; + } } /** @@ -459,8 +517,8 @@ gst_riff_parse_strf_auds (GstElement * element, * codec initialization data). * * Parses a interleaved (also known as "complex") stream´s strf - * structure plus optionally some extradata from input data. The - * input data is discarded after use. + * structure plus optionally some extradata from input data. This + * function takes ownership of @buf. * * Returns: TRUE if parsing succeeded, otherwise FALSE. */ @@ -471,13 +529,12 @@ gst_riff_parse_strf_iavs (GstElement * element, { gst_riff_strf_iavs *strf; - if (!buf || GST_BUFFER_SIZE (buf) < sizeof (gst_riff_strf_iavs)) { - GST_ERROR_OBJECT (element, - "Too small strf_iavs (%d available, %d needed)", - buf ? GST_BUFFER_SIZE (buf) : 0, (int) sizeof (gst_riff_strf_iavs)); - gst_buffer_unref (buf); - return FALSE; - } + g_return_val_if_fail (buf != NULL, FALSE); + g_return_val_if_fail (_strf != NULL, FALSE); + g_return_val_if_fail (data != NULL, FALSE); + + if (GST_BUFFER_SIZE (buf) < sizeof (gst_riff_strf_iavs)) + goto too_small; strf = g_memdup (GST_BUFFER_DATA (buf), GST_BUFFER_SIZE (buf)); gst_buffer_unref (buf); @@ -508,6 +565,16 @@ gst_riff_parse_strf_iavs (GstElement * element, *data = NULL; return TRUE; + + /* ERRORS */ +too_small: + { + GST_ERROR_OBJECT (element, + "Too small strf_iavs (%d available, %d needed)", + GST_BUFFER_SIZE (buf), sizeof (gst_riff_strf_iavs)); + gst_buffer_unref (buf); + return FALSE; + } } /** @@ -518,10 +585,8 @@ gst_riff_parse_strf_iavs (GstElement * element, * containing information about this stream. May be * NULL if no supported tags were found. * - * Parses stream metadata from input data. The input data is - * discarded after use. + * Parses stream metadata from input data. */ - void gst_riff_parse_info (GstElement * element, GstBuffer * buf, GstTagList ** _taglist) @@ -534,6 +599,9 @@ gst_riff_parse_info (GstElement * element, GstTagList *taglist; gboolean have_tags = FALSE; + g_return_if_fail (_taglist != NULL); + g_return_if_fail (buf != NULL); + if (!buf) { *_taglist = NULL; return; -- 2.34.1