On 06.11.2015 22:29, wm4 wrote:
> On Fri, 6 Nov 2015 22:18:04 +0100
> Andreas Cadhalpun <andreas.cadhal...@googlemail.com> wrote:
>
>> This fixes a stack buffer overflow.
>>
>> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
>> ---
>> libavcodec/pngdec.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
>> index 689aa2b..c974654 100644
>> --- a/libavcodec/pngdec.c
>> +++ b/libavcodec/pngdec.c
>> @@ -1010,13 +1010,13 @@ static int handle_p_frame_apng(AVCodecContext
>> *avctx, PNGDecContext *s,
>> memcpy(buffer + row_start, p->data[0] + row_start, s->bpp *
>> s->cur_w);
>> }
>> } else { // APNG_BLEND_OP_OVER
>> + uint8_t *output = av_malloc(s->bpp);
>> for (y = s->y_offset; y < s->y_offset + s->cur_h; ++y) {
>> uint8_t *foreground = p->data[0] + s->image_linesize * y +
>> s->bpp * s->x_offset;
>> uint8_t *background = buffer + s->image_linesize * y + s->bpp *
>> s->x_offset;
>> for (x = s->x_offset; x < s->x_offset + s->cur_w; ++x,
>> foreground += s->bpp, background += s->bpp) {
>> size_t b;
>> uint8_t foreground_alpha, background_alpha, output_alpha;
>> - uint8_t output[4];
>>
>> // Since we might be blending alpha onto alpha, we use the
>> following equations:
>> // output_alpha = foreground_alpha + (1 - foreground_alpha)
>> * background_alpha
>> @@ -1069,6 +1069,7 @@ static int handle_p_frame_apng(AVCodecContext *avctx,
>> PNGDecContext *s,
>> memcpy(background, output, s->bpp);
>> }
>> }
>> + av_freep(&output);
>> }
>>
>> // Copy blended buffer into the frame and free
>
> This seems wasteful, can't it just be output[8]?
I think s->bpp can be up to 10:
size_t byte_depth = s->bit_depth > 8 ? 2 : 1; // maximal 2
...
s->channels = ff_png_get_nb_channels(s->color_type); // maximal 4
s->bits_per_pixel = s->bit_depth * s->channels; // bit_depth
is maximal 16
s->bpp = (s->bits_per_pixel + 7) >> 3; // maximal 8
...
if (s->has_trns && s->color_type != PNG_COLOR_TYPE_PALETTE) {
...
s->bpp += byte_depth; // maximal 10
> It also adds a bug (unchecked malloc).
Right, sorry for that.
Attached is a patch increasing the buffer size to 10 and
adding an assert that s->bpp is not larger.
Best regards,
Andreas
>From bf724da5da5efe778225e61a786cc9b8cb86a91f Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Fri, 6 Nov 2015 23:44:01 +0100
Subject: [PATCH] apng: use correct size for output buffer
The buffer needs s->bpp bytes, at maximum currently 10.
Assert that s->bpp is not larger.
This fixes a stack buffer overflow.
Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
libavcodec/pngdec.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 689aa2b..feb1763 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -1016,7 +1016,7 @@ static int handle_p_frame_apng(AVCodecContext *avctx, PNGDecContext *s,
for (x = s->x_offset; x < s->x_offset + s->cur_w; ++x, foreground += s->bpp, background += s->bpp) {
size_t b;
uint8_t foreground_alpha, background_alpha, output_alpha;
- uint8_t output[4];
+ uint8_t output[10];
// Since we might be blending alpha onto alpha, we use the following equations:
// output_alpha = foreground_alpha + (1 - foreground_alpha) * background_alpha
@@ -1056,6 +1056,8 @@ static int handle_p_frame_apng(AVCodecContext *avctx, PNGDecContext *s,
output_alpha = foreground_alpha + FAST_DIV255((255 - foreground_alpha) * background_alpha);
+ av_assert0(s->bpp <= 10);
+
for (b = 0; b < s->bpp - 1; ++b) {
if (output_alpha == 0) {
output[b] = 0;
--
2.6.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
