[FFmpeg-devel] [PATCH 1/3] tiff: port to bytestream2

Michael Niedermayer michaelni at gmx.at
Mon Jul 16 21:42:48 CEST 2012


On Mon, Jul 16, 2012 at 01:03:06AM +0000, Paul B Mahol wrote:
> Prevents out of array reads.
> 
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
>  libavcodec/tiff.c |  217 +++++++++++++++++++++++++++--------------------------
>  1 files changed, 112 insertions(+), 105 deletions(-)
> 
> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> index 88f5238..2cc960a 100644
> --- a/libavcodec/tiff.c
> +++ b/libavcodec/tiff.c
> @@ -25,6 +25,7 @@
>   */
>  
>  #include "avcodec.h"
> +#include "bytestream.h"
>  #if CONFIG_ZLIB
>  #include <zlib.h>
>  #endif
> @@ -40,6 +41,7 @@
>  typedef struct TiffContext {
>      AVCodecContext *avctx;
>      AVFrame picture;
> +    GetByteContext gb;
>  
>      int width, height;
>      unsigned int bpp, bppcount;
> @@ -54,42 +56,37 @@ typedef struct TiffContext {
>  
>      int strips, rps, sstype;
>      int sot;
> -    const uint8_t *stripdata;
> -    const uint8_t *stripsizes;
> -    int stripsize, stripoff;
> +    int stripsizesoff, stripsize, stripoff, strippos;
>      LZWState *lzw;
>  
>      int geotag_count;
>      TiffGeoTag *geotags;
>  } TiffContext;
>  
> -static unsigned tget_short(const uint8_t **p, int le)
> +static unsigned tget_short(GetByteContext *gb, int le)
>  {
> -    unsigned v = le ? AV_RL16(*p) : AV_RB16(*p);
> -    *p += 2;
> +    unsigned v = le ? bytestream2_get_le16(gb) : bytestream2_get_be16(gb);
>      return v;
>  }
>  
> -static unsigned tget_long(const uint8_t **p, int le)
> +static unsigned tget_long(GetByteContext *gb, int le)
>  {
> -    unsigned v = le ? AV_RL32(*p) : AV_RB32(*p);
> -    *p += 4;
> +    unsigned v = le ? bytestream2_get_le32(gb) : bytestream2_get_be32(gb);
>      return v;
>  }
>  
> -static double tget_double(const uint8_t **p, int le)
> +static double tget_double(GetByteContext *gb, int le)
>  {
> -    av_alias64 i = { .u64 = le ? AV_RL64(*p) : AV_RB64(*p)};
> -    *p += 8;
> +    av_alias64 i = { .u64 = le ? bytestream2_get_le64(gb) : bytestream2_get_be64(gb)};
>      return i.f64;
>  }
>  
> -static unsigned tget(const uint8_t **p, int type, int le)
> +static unsigned tget(GetByteContext *gb, int type, int le)
>  {
>      switch (type) {
> -    case TIFF_BYTE : return *(*p)++;
> -    case TIFF_SHORT: return tget_short(p, le);
> -    case TIFF_LONG : return tget_long(p, le);
> +    case TIFF_BYTE : return bytestream2_get_byte(gb);
> +    case TIFF_SHORT: return tget_short(gb, le);
> +    case TIFF_LONG : return tget_long(gb, le);
>      default        : return UINT_MAX;
>      }
>  }
> @@ -223,7 +220,7 @@ static char *doubles2str(double *dp, int count, const char *sep)
>      return ap0;
>  }
>  
> -static char *shorts2str(int *sp, int count, const char *sep)
> +static char *shorts2str(int16_t *sp, int count, const char *sep)
>  {
>      int i;
>      char *ap, *ap0;
> @@ -241,18 +238,23 @@ static char *shorts2str(int *sp, int count, const char *sep)
>      return ap0;
>  }
>  
> -static int add_doubles_metadata(const uint8_t **buf, int count,
> +static int add_doubles_metadata(int count,
>                                  const char *name, const char *sep,
>                                  TiffContext *s)
>  {
>      char *ap;
>      int i;
> -    double *dp = av_malloc(count * sizeof(double));
> +    double *dp;
> +


> +    if (bytestream2_get_bytes_left(&s->gb) < count * sizeof(int64_t))
> +        return -1;

missing interger overflow check


> +
> +    dp = av_malloc(count * sizeof(double));
>      if (!dp)
>          return AVERROR(ENOMEM);
>  
>      for (i = 0; i < count; i++)
> -        dp[i] = tget_double(buf, s->le);
> +        dp[i] = tget_double(&s->gb, s->le);
>      ap = doubles2str(dp, count, sep);
>      av_freep(&dp);
>      if (!ap)
> @@ -261,17 +263,22 @@ static int add_doubles_metadata(const uint8_t **buf, int count,
>      return 0;
>  }
>  
> -static int add_shorts_metadata(const uint8_t **buf, int count, const char *name,
> +static int add_shorts_metadata(int count, const char *name,
>                                 const char *sep, TiffContext *s)
>  {
>      char *ap;
>      int i;
> -    int *sp = av_malloc(count * sizeof(int));
> +    int16_t *sp;
> +

> +    if (bytestream2_get_bytes_left(&s->gb) < count * sizeof(int16_t))
> +        return -1;

also missing integer overflwo check


otherwise LGTM

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120716/e0fb3803/attachment.asc>


More information about the ffmpeg-devel mailing list