On Fri, 6 Nov 2015 23:56:52 +0100
Andreas Cadhalpun <andreas.cadhal...@googlemail.com> wrote:
> 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.
I'm find with this, though I'm not (A)PNG maintainer.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
