[FFmpeg-devel] [PATCH] gifdec: use truncated width for image manipulation

[Michael Niedermayer]

On Sun, Aug 17, 2014 at 10:41:53AM +0200, Christophe Gisquet wrote:
> Hi,
> 
> the attached patch fixes ticket #3538, which is an off-by-one error.
> Unfortunately, I see no way of detecting it as a "correctable"
> behavior and not an actual error besides that.
> 
> Maybe restricting this to actual off-by-one errors would be better, too.
> 
> -- 
> Christophe

>  gifdec.c |   31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 7200a543395ab7e6748dc1136b0a21d3678fdf72  0001-gifdec-use-truncated-width-for-image-manipulation.patch
> From 59ea1d72b27272d2a28c680fc87b353f1a88eb36 Mon Sep 17 00:00:00 2001
> From: Christophe Gisquet <christophe.gisquet at gmail.com>
> Date: Sun, 17 Aug 2014 09:47:46 +0200
> Subject: [PATCH] gifdec: use truncated width for image manipulation
> 
> Some files seem to have an off-by-one error. In most cases, it appears to
> be on the image width. Therefore, if the decoded image doesn't fit in the
> screen:
> - If it is wider than the screen (and the lzw decoding buffer), reject it;
> - Otherwise, decode the indicated amount, but only write a truncated amount
>   to the screen.
> 
> Fixes ticket #3538.
> ---
>  libavcodec/gifdec.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/gifdec.c b/libavcodec/gifdec.c
> index 78c8900..22da582 100644
> --- a/libavcodec/gifdec.c
> +++ b/libavcodec/gifdec.c
> @@ -129,7 +129,7 @@ static void gif_copy_img_rect(const uint32_t *src, uint32_t *dst,
>  
>  static int gif_read_image(GifState *s, AVFrame *frame)
>  {
> -    int left, top, width, height, bits_per_pixel, code_size, flags;
> +    int left, top, width, height, bits_per_pixel, code_size, flags, pw;
>      int is_interleaved, has_local_palette, y, pass, y1, linesize, pal_size;
>      uint32_t *ptr, *pal, *px, *pr, *ptr1;
>      int ret;
> @@ -179,15 +179,28 @@ static int gif_read_image(GifState *s, AVFrame *frame)
>      }
>  
>      /* verify that all the image is inside the screen dimensions */
> -    if (left + width > s->screen_width ||
> -        top + height > s->screen_height) {
> -        av_log(s->avctx, AV_LOG_ERROR, "image is outside the screen dimensions.\n");
> -        return AVERROR_INVALIDDATA;
> -    }
>      if (width <= 0 || height <= 0) {
>          av_log(s->avctx, AV_LOG_ERROR, "Invalid image dimensions.\n");
>          return AVERROR_INVALIDDATA;
>      }
> +    if (width > s->screen_width) {
> +        av_log(s->avctx, AV_LOG_ERROR, "Invalid image width.\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +    if (left + width > s->screen_width) {
> +        /* width must be kept around to avoid lzw vs line desync */
> +        pw = s->screen_width - left;
> +        av_log(s->avctx, AV_LOG_WARNING, "Image too wide by %d, truncating.\n",
> +               left + width - s->screen_width);
> +    } else {
> +        pw = width;
> +    }
> +    if (top + height > s->screen_height) {
> +        /* we don't care about the extra invisible lines */
> +        av_log(s->avctx, AV_LOG_WARNING, "Image too high by %d, truncating.\n",
> +               top + height - s->screen_height);
> +        height = s->screen_height - top;
> +    }

i think these need a check for top >= s->screen_height and
left >= s->screen_width

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140817/42bc4c74/attachment.asc>