diff mbox

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

Message ID 20181214214407.58880-1-chcunningham@chromium.org
State Accepted
Commit ee1e39a576977fd38c3b94fc56125d31d38833e9
Headers show

Commit Message

chcunningham@chromium.org Dec. 14, 2018, 9:44 p.m. UTC 
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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Tomas Härdin Dec. 16, 2018, 8:42 p.m. UTC | #1 
fre 2018-12-14 klockan 13:44 -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 | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> index f7de26a1d8..5fe055b591 100644
> --- a/libavformat/id3v2.c
> +++ b/libavformat/id3v2.c
> @@ -590,7 +590,7 @@ static void read_apic(AVFormatContext *s,
> AVIOContext *pb, int taglen,
>                        int isv34)
>  {
>      int enc, pic_type;
> -    char mimetype[64];
> +    char mimetype[64] = {0};
>      const CodecMime *mime     = ff_id3v2_mime_tags;
>      enum AVCodecID id         = AV_CODEC_ID_NONE;
>      ID3v2ExtraMetaAPIC *apic  = NULL;
> @@ -612,7 +612,9 @@ static void read_apic(AVFormatContext *s,
> AVIOContext *pb, int taglen,
>      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;
> +

Looks good to me

/Tomas
Michael Niedermayer Dec. 17, 2018, 5:41 p.m. UTC | #2 
On Sun, Dec 16, 2018 at 09:42:49PM +0100, Tomas Härdin wrote:
> fre 2018-12-14 klockan 13:44 -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 | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> > index f7de26a1d8..5fe055b591 100644
> > --- a/libavformat/id3v2.c
> > +++ b/libavformat/id3v2.c
> > @@ -590,7 +590,7 @@ static void read_apic(AVFormatContext *s,
> > AVIOContext *pb, int taglen,
> >                        int isv34)
> >  {
> >      int enc, pic_type;
> > -    char mimetype[64];
> > +    char mimetype[64] = {0};
> >      const CodecMime *mime     = ff_id3v2_mime_tags;
> >      enum AVCodecID id         = AV_CODEC_ID_NONE;
> >      ID3v2ExtraMetaAPIC *apic  = NULL;
> > @@ -612,7 +612,9 @@ static void read_apic(AVFormatContext *s,
> > AVIOContext *pb, int taglen,
> >      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;
> > +
> 
> Looks good to me

will apply

thx

[...]
diff mbox

Patch

diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
index f7de26a1d8..5fe055b591 100644
--- a/libavformat/id3v2.c
+++ b/libavformat/id3v2.c
@@ -590,7 +590,7 @@  static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen,
                       int isv34)
 {
     int enc, pic_type;
-    char mimetype[64];
+    char mimetype[64] = {0};
     const CodecMime *mime     = ff_id3v2_mime_tags;
     enum AVCodecID id         = AV_CODEC_ID_NONE;
     ID3v2ExtraMetaAPIC *apic  = NULL;
@@ -612,7 +612,9 @@  static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen,
     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;
     }