On Sun, Mar 29, 2015 at 11:05:40AM +0000, Donny Yang wrote:
> Signed-off-by: Donny Yang <w...@kota.moe>
> ---
>  libavcodec/pngenc.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
> index d6233d0..3697dbb 100644
> --- a/libavcodec/pngenc.c
> +++ b/libavcodec/pngenc.c
> @@ -360,12 +360,23 @@ static int encode_frame(AVCodecContext *avctx, AVPacket 
> *pkt,
>          return -1;
>  
>      enc_row_size    = deflateBound(&s->zstream, row_size);
> -    max_packet_size = avctx->height * (int64_t)(enc_row_size +
> -                                       ((enc_row_size + IOBUF_SIZE - 1) / 
> IOBUF_SIZE) * 12)
> -                      + FF_MIN_BUFFER_SIZE;
> +    max_packet_size =
> +        8 +             // PNGSIG
> +        13 + 12 +       // IHDR
> +        9 + 12 +        // pHYs
> +        1 + 12 +        // sRGB
> +        32 + 12 +       // cHRM
> +        4 + 12 +        // gAMA
> +        256 * 3 + 12 +  // PLTE
> +        256 + 12 +      // tRNS
> +        avctx->height * (
> +            enc_row_size +
> +            12 * (((int64_t)enc_row_size + IOBUF_SIZE - 1) / IOBUF_SIZE) // 
> 12 * ceil(enc_row_size / IOBUF_SIZE)
> +        );
>      if (max_packet_size > INT_MAX)
>          return AVERROR(ENOMEM);
> -    if ((ret = ff_alloc_packet2(avctx, pkt, max_packet_size)) < 0)
> +    ret = ff_alloc_packet2(avctx, pkt, max_packet_size < FF_MIN_BUFFER_SIZE 
> ? FF_MIN_BUFFER_SIZE : max_packet_size);

i understand the change is well meant and it makes the allocated size
closer to what is actually written but
this would require the list of sizes to be kept in sync with what is
actually written. This is a additional maintaince burden and has
serious consequence if one makes a change and then forgets updating
the size as it could result in out of array writes becoming possible
and then theres a possibly exploitable security issues if that happens

so i suggest to leave the "+ FF_MIN_BUFFER_SIZE" in there unless there
is something else that protects against more data being written than
what is listed here


> +    if (ret)

(ret < 0)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.

Attachment: signature.asc
Description: Digital signature

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

Reply via email to