[FFmpeg-devel] [PATCH] IFF: Add error checking to byterun1 decoder

[Ronald S. Bultje]

Hi,

On Sun, May 23, 2010 at 11:31 AM, Sebastian Vater
<cdgs.basty at googlemail.com> wrote:
> Fixed. For preventing the function become non-inlined then I have added
> av_always_inline to it. I also pass AVCodecContext
> to it.

The inling should be in a separate patch.

> @@ -226,27 +226,43 @@ static void decodeplane32(uint32_t *dst, const uint8_t *buf, int buf_size, int p
>          const int8_t value = *buf++;
>          if (value >= 0) {
> -            length = value + 1;
> -            memcpy(dst + x, buf, FFMIN3(length, dst_size - x, buf_end - buf));
> +            const int length = (unsigned) value + 1;
> +            if (length > dst_size || length > (int) (buf_end - buf)) { // overflow?
> +                av_log(avctx, AV_LOG_ERROR, "IFF byterun1 stream buffer overflow\n");
> +                return AVERROR_INVALIDDATA;
> +            }
> +            memcpy(dst, buf, length);
> +            dst_size -= length;
> +            dst += length;
>              buf += length;
[..]
>          } else if (value > -128) {
> -            length = -value + 1;
> -            memset(dst + x, *buf++, FFMIN(length, dst_size - x));
> -        } else { // noop
> -            continue;
> +            const int length = (unsigned) -value + 1;
> +            if (length > dst_size || buf >= buf_end) { // overflow?
> +                av_log(avctx, AV_LOG_ERROR, "IFF byterun1 stream buffer overflow\n");
> +                return AVERROR_INVALIDDATA;
> +            }
> +            memset(dst, *buf++, length);
> +            dst_size -= length;
> +            dst += length;
> +        } else if (buf >= buf_end) { // noop, return error on overflow, though
> +            av_log(avctx, AV_LOG_ERROR, "IFF byterun1 stream buffer overflow\n");
> +            return AVERROR_INVALIDDATA;
>          }
> -        x += length;
> -    }
> +    } while (dst_size > 0);
>      return buf - buf_start;
>  }

IMO this does a lot more than just adding error checking? Is this
faster? Or just different?

Ronald