[FFmpeg-devel] [PATCH] mss2: Fix buffer overflow.

wm4 nfxjfg at googlemail.com
Thu Feb 25 21:25:08 CET 2016


On Thu, 25 Feb 2016 21:06:46 +0100
Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:

> Reported as https://trac.mplayerhq.hu/ticket/2264 but have
> not been able to reproduce with FFmpeg-only.
> I have no idea what coded_height is used for here exactly,
> so this might not be the best fix.
> Fixes the following chain of events:
> ff_mss12_decode_init sets coded_height while not setting height.
> ff_mpv_decode_init then copies coded_height into MpegEncContext height.
> This is then used by init_context_frame to allocate the data structures.
> However the wmv9rects are validated/initialized based on avctx->height, not
> avctx->coded_height.
> Thus the decode_wmv9 function will try to decode a larger video that we
> allocated data structures for, causing out-of-bounds writes.
> 
> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> ---
>  libavcodec/mss12.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/mss12.c b/libavcodec/mss12.c
> index 6b58aa29..d42093b 100644
> --- a/libavcodec/mss12.c
> +++ b/libavcodec/mss12.c
> @@ -581,8 +581,8 @@ av_cold int ff_mss12_decode_init(MSS12Context *c, int version,
>          return AVERROR_INVALIDDATA;
>      }
>  
> -    avctx->coded_width  = AV_RB32(avctx->extradata + 20);
> -    avctx->coded_height = AV_RB32(avctx->extradata + 24);
> +    avctx->coded_width  = FFMAX(AV_RB32(avctx->extradata + 20), avctx->width);
> +    avctx->coded_height = FFMAX(AV_RB32(avctx->extradata + 24), avctx->height);
>      if (avctx->coded_width > 4096 || avctx->coded_height > 4096) {
>          av_log(avctx, AV_LOG_ERROR, "Frame dimensions %dx%d too large",
>                 avctx->coded_width, avctx->coded_height);

Just a side remark...

I'm wondering why ff_set_dimensions doesn't have coded_width/height
arguments and checks them. I bet this situation happens often.


More information about the ffmpeg-devel mailing list