[FFmpeg-devel] [PATCH] webp: ensure that each transform is only used once

[Michael Niedermayer]

On Thu, Mar 05, 2015 at 11:02:20PM +0100, Andreas Cadhalpun wrote:
> Hi,
> 
> according to the WebP Lossless Bitstream Specification [1] "each
> transform is allowed to be used only once". Attached patch adds
> checks for this to avoid crashes decoding broken files.
> 
> Best regards,
> Andreas
> 
> 1: https://developers.google.com/speed/webp/docs/webp_lossless_bitstream_specification#3_transformations

>  webp.c |   39 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 75151057a49b79fb058771e0f9e59d8fd68bd5c2  0001-webp-ensure-that-each-transform-is-only-used-once.patch
> From d80baa7f786ca326891e145a000fbecdde55da80 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Thu, 5 Mar 2015 22:48:28 +0100
> Subject: [PATCH] webp: ensure that each transform is only used once
> 
> According to the WebP Lossless Bitstream Specification
> "each transform is allowed to be used only once".
> 
> If a transform is more than once this can lead to memory
> corruption.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> ---
>  libavcodec/webp.c | 39 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/webp.c b/libavcodec/webp.c
> index 9549c0e..2cd976f 100644
> --- a/libavcodec/webp.c
> +++ b/libavcodec/webp.c
> @@ -1104,7 +1104,7 @@ static int vp8_lossless_decode_frame(AVCodecContext *avctx, AVFrame *p,
>                                       unsigned int data_size, int is_alpha_chunk)
>  {
>      WebPContext *s = avctx->priv_data;
> -    int w, h, ret, i;
> +    int w, h, ret, i, used;
>  
>      if (!is_alpha_chunk) {
>          s->lossless = 1;
> @@ -1154,18 +1154,49 @@ static int vp8_lossless_decode_frame(AVCodecContext *avctx, AVFrame *p,
>      /* parse transformations */
>      s->nb_transforms = 0;
>      s->reduced_width = 0;
> +    used = 0;
>      while (get_bits1(&s->gb)) {
>          enum TransformType transform = get_bits(&s->gb, 2);
>          s->transforms[s->nb_transforms++] = transform;
>          switch (transform) {
>          case PREDICTOR_TRANSFORM:
> -            ret = parse_transform_predictor(s);
> +            if (used & 1) {
> +                av_log(avctx, AV_LOG_ERROR,
> +                       "Predictor transform used more than once\n");
> +                ret = AVERROR_INVALIDDATA;
> +            } else {
> +                used |= 1;
> +                ret = parse_transform_predictor(s);
> +            }

can be simplified to something like

if (used & (1<<transform)) {
    av_log(avctx, AV_LOG_ERROR, "%s used more than once\n", name[transform]);
    ret = AVERROR_INVALIDDATA;
    goto free_and_return;
}
used |= 1<<transform;

or
if(used[transform]++)
even

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- 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/20150306/1b9a89c8/attachment.asc>