Web lists-archives.com

Re: [MPlayer-dev-eng] [PATCH] spudec: fix heap overflow in pal2gray_alpha()




On Tue, Jul 01, 2014 at 03:45:31PM +0200, Matthijs van Otterdijk wrote:
> sub/spudec.c:spudec_packet_fill() optionally draws rectangles with an x and
> y offset, which is used by sub/av_sub.c:avsub_to_spudec() in case of
> multiple rects. The way this is done now causes a heap overflow in
> spudec.c:pal2gray_alpha().
> spudec_packet_fill() offsets img and aimg by x before calling
> pal2gray_alpha(). pal2gray_alpha() writes dst_stride pixels for each line
> in the rect. In case the bottom rectangle (and therefore the rectangle
> situated at the end of the packet buffer) has an x offset, this will cause
> x 0s to be written past the end of the packet buffer.
> 
> The attached patch fixes this by making pal2gray_alpha() handle the x
> offset, rather than spudec_packet_fill().

Really sorry for the long delay, with holidays in-between I forgot about
it.
Is there an easy test-case?
I think this change is wrong (or at least, it still leaves issues).
It means that if there are two subtitle rects next to each other,
only one will ever be visible.
I think instead we need to skip the clearing part (avsub_to_spudec
will pre-clear the image anyway).
I would welcome testing of attached patch.

Regards,
Reimar
diff --git a/sub/spudec.c b/sub/spudec.c
index ea5cbdb..836fcab 100644
--- a/sub/spudec.c
+++ b/sub/spudec.c
@@ -254,7 +254,7 @@ static int spudec_alloc_image(spudec_handle_t *this, int stride, int height)
 static void pal2gray_alpha(const uint16_t *pal,
                            const uint8_t *src, int src_stride,
                            uint8_t *dst, uint8_t *dsta,
-                           int dst_stride, int w, int h)
+                           int dst_stride, int w, int h, int skip_stride)
 {
   int x, y;
   for (y = 0; y < h; y++) {
@@ -263,7 +263,10 @@ static void pal2gray_alpha(const uint16_t *pal,
       *dst++  = pixel;
       *dsta++ = pixel >> 8;
     }
-    for (; x < dst_stride; x++)
+    if (skip_stride) {
+      dst += dst_stride - w;
+      dsta += dst_stride - w;
+    } else for (; x < dst_stride; x++)
       *dsta++ = *dst++ = 0;
     src += src_stride;
   }
@@ -306,7 +309,7 @@ static int apply_palette_crop(spudec_handle_t *this,
   src = this->pal_image + crop_y * this->pal_width + crop_x;
   pal2gray_alpha(pal, src, this->pal_width,
                  this->image, this->aimage, stride,
-                 crop_w, crop_h);
+                 crop_w, crop_h, 0);
   this->width  = crop_w;
   this->height = crop_h;
   this->stride = stride;
@@ -1419,7 +1422,7 @@ void spudec_packet_fill(packet_t *packet,
       g8a8_pal[i] = (-alpha << 8) | gray;
   }
   pal2gray_alpha(g8a8_pal, pal_img, pal_stride,
-                 img, aimg, packet->stride, w, h);
+                 img, aimg, packet->stride, w, h, 1);
 }
 
 void spudec_packet_send(void *spu, packet_t *packet, double pts, double endpts)
_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-eng@xxxxxxxxxxxx
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng