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

[Michael Niedermayer]

On Fri, Mar 06, 2015 at 12:20:06AM +0100, Andreas Cadhalpun wrote:
> On 05.03.2015 23:12, Justin Ruggles wrote:
> >On 03/05/2015 05:02 PM, Andreas Cadhalpun wrote:
> >>>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.
> >
> >Can you give more details about the memory corruption?
> 
> Yes, but I think it's not really important, because the code was
> never meant to handle the same transform twice.
> Anyway: If e.g. the color indexing transform is used twice, then
> parse_transform_color_indexing is called twice, which calls
> decode_entropy_coded_image with different parameters w.
> There av_frame_get_buffer(img->frame, 1) is called, which the first
> time allocates a buffer for the first w, but the second time,
> img->frame->linesize[0] is not 0, so it is not updated in
> get_video_buffer, which means a buffer of the wrong size (e.g. too
> small) is allocated.
> When this buffer is later written to, it just writes after the end
> of the buffer, overwriting whatever is there, e.g. pointers to other
> buffers, which then leads to crashes if these are accessed/freed.
> 
> >>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) {
> >
> >Use (1 << transform) and move all of these checks to a single check
> >before the switch.
> 
> That's a good idea. Update patch attached.
> 
> Best regards,
> Andreas
> 

>  webp.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 9614f4dbed6f3bc6c8902afddf1d079b0d87419f  0001-webp-ensure-that-each-transform-is-only-used-once.patch
> From de11cf3ffbdf9d5c2118e42b8c0e59297c7e28ee 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>

applied

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- 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/9941f279/attachment.asc>