[FFmpeg-devel] lavf/id3v2: fail read_apic on EOF reading mimetype

Submitted by chcunningham@chromium.org on Dec. 13, 2018, 7:28 p.m.

Details

Message ID 20181213192826.92374-1-chcunningham@chromium.org
State New
Headers show

Commit Message

chcunningham@chromium.org Dec. 13, 2018, 7:28 p.m.
avio_read may return EOF, leaving the mimetype array unitialized. fail
early when this occurs to avoid using the array in an unitialized state.
---
 libavformat/id3v2.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Tomas Härdin Dec. 14, 2018, 11:27 a.m.
tor 2018-12-13 klockan 11:28 -0800 skrev chcunningham:
> avio_read may return EOF, leaving the mimetype array unitialized. fail
> early when this occurs to avoid using the array in an unitialized state.
> ---
>  libavformat/id3v2.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> index f7de26a1d8..f06b9a787e 100644
> --- a/libavformat/id3v2.c
> +++ b/libavformat/id3v2.c
> @@ -609,10 +609,13 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen,
>      taglen--;
>  
>      /* mimetype */
> +    memset(mimetype, 0, sizeof(mimetype));

-char mimetype[64];
+char mimetype[64] = {0};

would be enough

/Tomas
chcunningham@chromium.org Dec. 14, 2018, 8:22 p.m.
>
> -char mimetype[64];
> +char mimetype[64] = {0};
>
> would be enough
>

Agree! Next patch.

I'd love to do similar for the isv34 branch, but I'm not sure how to detect
> the condition given the EOF behavior for  avio_get_str:
>  * @return number of bytes read (is always <= maxlen).
>  * If reading ends on EOF or error, the return value will be one more than
>  * bytes actually read.
> How do callers differentiate between cases where you read 5 bytes vs
> reading just 4 bytes and hitting an error - IIIUC both cases return 5.


Anyone familiar with this? Seems like a bad way to signal EOF.

Patch hide | download patch | download mbox

diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
index f7de26a1d8..f06b9a787e 100644
--- a/libavformat/id3v2.c
+++ b/libavformat/id3v2.c
@@ -609,10 +609,13 @@  static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen,
     taglen--;
 
     /* mimetype */
+    memset(mimetype, 0, sizeof(mimetype));
     if (isv34) {
         taglen -= avio_get_str(pb, taglen, mimetype, sizeof(mimetype));
     } else {
-        avio_read(pb, mimetype, 3);
+        if (avio_read(pb, mimetype, 3) < 0)
+            goto fail;
+
         mimetype[3] = 0;
         taglen    -= 3;
     }