[FFmpeg-devel] [PATCH] buffer read sanity check (issue 2503)

[Michael Niedermayer]

On Thu, Jan 06, 2011 at 11:51:58PM -0500, Daniel Kang wrote:
> ffmpeg does not check for buffer overreading in the dpx decoder. For
> invalid headers, the target_packet_size or width could be too large. The
> patch attached adds a sanity check.

>  dpx.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> e3c6cd5042d736935c2c5a730c93ee96f108b988  dpx_buffer_sanity_check.diff
> From ae6ec1ce796735a666afb6b911c8c9e594b1eb8a Mon Sep 17 00:00:00 2001
> From: Daniel Kang <daniel.d.kang at gmail.com>
> Date: Thu, 6 Jan 2011 23:34:05 -0500
> Subject: [PATCH] dpx buffer overread sanity check
> 
> ---
>  libavcodec/dpx.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c
> index f92b3d0..588ec6c 100644
> --- a/libavcodec/dpx.c
> +++ b/libavcodec/dpx.c
> @@ -55,6 +55,7 @@ static int decode_frame(AVCodecContext *avctx,
>                          AVPacket *avpkt)
>  {
>      const uint8_t *buf = avpkt->data;
> +    const uint8_t *buf_end = avpkt->data + avpkt->size;
>      int buf_size       = avpkt->size;
>      DPXContext *const s = avctx->priv_data;
>      AVFrame *picture  = data;
> @@ -174,6 +175,10 @@ static int decode_frame(AVCodecContext *avctx,
>          case 16:
>              if (source_packet_size == target_packet_size) {
>                  for (x = 0; x < avctx->height; x++) {
> +                    if (buf + target_packet_size*avctx->width > buf_end) {
> +                        av_log(avctx, AV_LOG_ERROR, "Overread buffer. Invalid header?\n");
> +                        return -1;
> +                    }
>                      memcpy(ptr, buf, target_packet_size*avctx->width);
>                      ptr += stride;
>                      buf += source_packet_size*avctx->width;
> @@ -182,6 +187,10 @@ static int decode_frame(AVCodecContext *avctx,
>                  for (x = 0; x < avctx->height; x++) {
>                      uint8_t *dst = ptr;
>                      for (y = 0; y < avctx->width; y++) {
> +                        if (buf + target_packet_size > buf_end) {
> +                            av_log(avctx, AV_LOG_ERROR, "Overread buffer. Invalid header?\n");
> +                            return -1;
> +                        }

something like

if(source_packet_size * avctx->width > buf_end - buf)
at a single spot should be able to catch all i think

[...]
--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110107/e8ae4415/attachment.pgp>