[FFmpeg-devel] [PATCH] 2/6 Fix broken APE tag key handling

[Kostya]

On Mon, Aug 10, 2009 at 12:34:16PM +0300, Matti Hamalainen wrote:
> 
> Fix broken APE tag key handling and use get_byte instead of dangerously
> fiddling with the ByteIOContext buffer directly without bounds checking.
> Also add return value to ape_tag_read_field() and break on reading further
> tag fields if one fails.
> 
> -- 
> ] ccr/TNSP^DKD^pWp  :: ccr tnsp org ::  http://ccr.tnsp.org/
> ] PGP key: 0466 95ED 96DF 3701 C71D D62D 10A6 28A6 1374 C112
> # HG changeset patch
> # User Matti Hamalainen <ccr at tnsp.org>
> # Date 1249795469 -10800
> # Node ID 5b9601f406caf2cce8f4d080876b7cac2aed61e4
> # Parent  ad8ef14eecc604568386d4461f7b217d45b646eb
> Fix broken APE tag key handling and use get_byte instead of dangerously
> fiddling with the ByteIOContext buffer directly without bounds checking.
> Also add return value to ape_tag_read_field() and break on reading further
> tag fields if one fails.
> 
> diff -r ad8ef14eecc6 -r 5b9601f406ca libavformat/ape.c
> --- a/libavformat/ape.c	Sun Aug 09 08:16:18 2009 +0300
> +++ b/libavformat/ape.c	Sun Aug 09 08:24:29 2009 +0300
> @@ -91,22 +91,27 @@
>      uint32_t *seektable;
>  } APEContext;
>  
> -static void ape_tag_read_field(AVFormatContext *s)
> +static int ape_tag_read_field(AVFormatContext *s)
>  {
>      ByteIOContext *pb = s->pb;
>      uint8_t key[1024], value[1024];
> -    uint32_t size;
> -    int i, l;
> +    uint32_t size, flags;
> +    int i, l, c;
>  
>      size = get_le32(pb);  /* field size */
> -    url_fskip(pb, 4);     /* skip field flags */
> -
> -    for (i=0; pb->buf_ptr[i]!='0' && pb->buf_ptr[i]>=0x20 && pb->buf_ptr[i]<=0x7E; i++);

Ahem, is this buf_ptr[i]!='0' an error by original author or you just
forgot to check for it?

> -
> -    l = FFMIN(i,    sizeof(key) -1);
> -    get_buffer(pb, key,  l);
> -    key[l]  = 0;
> -    url_fskip(pb, 1 + i-l);
> +    flags = get_le32(pb); /* field flags */
> +    for (i = 0; i < sizeof(key) - 1; i++) {
> +        c = get_byte(pb);
> +        if (c < 0x20 || c > 0x7E)
> +            break;
> +        else
> +            key[i] = c;
> +    }
> +    key[i] = 0;
> +    if (c != 0) {
> +        av_log(s, AV_LOG_WARNING, "Invalid APE tag key '%s'.\n", key);
> +        return -1;
> +    }
>      l = FFMIN(size, sizeof(value)-1);
>      get_buffer(pb, value, l);
>      value[l] = 0;
> @@ -114,6 +119,7 @@
>      if (l < size)
>          av_log(s, AV_LOG_WARNING, "Too long '%s' tag was truncated.\n", key);
>      av_metadata_set(&s->metadata, key, value);
> +    return 0;
>  }
>  
>  static void ape_parse_tag(AVFormatContext *s)
> @@ -161,7 +167,7 @@
>      url_fseek(pb, file_size - tag_bytes, SEEK_SET);
>  
>      for (i=0; i<fields; i++)
> -        ape_tag_read_field(s);
> +        if (ape_tag_read_field(s) < 0) break;
>  
>  #if ENABLE_DEBUG
>      av_log(s, AV_LOG_DEBUG, "\nAPE Tags:\n\n");

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel