On 03.05.2015 20:09, Michael Niedermayer wrote:
> On Sun, May 03, 2015 at 05:11:17PM +0200, Andreas Cadhalpun wrote:
>> In this case ptr could be set to a position outside the image_buf in
>> png_handle_row, leading to memory corruption and thus crashes.
>>
>> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
>> ---
>>  libavcodec/pngdec.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
>> index 26de09d..4488726 100644
>> --- a/libavcodec/pngdec.c
>> +++ b/libavcodec/pngdec.c
>> @@ -655,6 +655,12 @@ static int decode_idat_chunk(AVCodecContext *avctx, 
>> PNGDecContext *s,
>>                  s->row_size, s->crow_size);
>>          s->image_buf      = p->data[0];
>>          s->image_linesize = p->linesize[0];
>> +        if (s->x_offset * s->bpp >= s->image_linesize) {
> 
> this doesnt look correct, the linesize could be larger than the
> width in pixels yet still x_offset shouldnt point outside the width
> and x_offset is being checked where its read

The problem is that the original check assumes linesize >= width.
However this is not the case for AV_PIX_FMT_MONOBLACK, which has the
AV_PIX_FMT_FLAG_BITSTREAM flag. Thus the linesize is divided by 8 in
image_get_linesize:
    if (desc->flags & AV_PIX_FMT_FLAG_BITSTREAM)
        linesize = (linesize + 7) >> 3;

> maybe theres some missing check on chunk order or something changes
> thats assumed not to ...

Alternatively one could just not accept AV_PIX_FMT_MONOBLACK in
combination with AV_CODEC_ID_APNG. Patch for that attached.

Best regards,
Andreas

>From e93b91faa3e6594a254dda43d0c54f47243ac7b4 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Sun, 3 May 2015 20:36:20 +0200
Subject: [PATCH] pngdec: don't use AV_PIX_FMT_MONOBLACK for apng

AV_PIX_FMT_MONOBLACK has the AV_PIX_FMT_FLAG_BITSTREAM flag, i.e.
linesize can be smaller than width.

Since x_offset is only check against the width, this can lead to
x_offset * bpp >= image_linesize.

In this case ptr could be set to a position outside the image_buf in
png_handle_row, leading to memory corruption and thus crashes.

Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
 libavcodec/pngdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 26de09d..ca3b9c8 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -618,7 +618,7 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
         } else if ((s->bits_per_pixel == 1 || s->bits_per_pixel == 2 || s->bits_per_pixel == 4 || s->bits_per_pixel == 8) &&
                 s->color_type == PNG_COLOR_TYPE_PALETTE) {
             avctx->pix_fmt = AV_PIX_FMT_PAL8;
-        } else if (s->bit_depth == 1 && s->bits_per_pixel == 1) {
+        } else if (s->bit_depth == 1 && s->bits_per_pixel == 1 && avctx->codec_id != AV_CODEC_ID_APNG) {
             avctx->pix_fmt = AV_PIX_FMT_MONOBLACK;
         } else if (s->bit_depth == 8 &&
                 s->color_type == PNG_COLOR_TYPE_GRAY_ALPHA) {
-- 
2.1.4

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to