On Thu, Nov 24, 2016 at 02:14:59AM +0100, Andreas Cadhalpun wrote:
> On 23.11.2016 15:01, Michael Niedermayer wrote:
> > On Mon, Nov 07, 2016 at 10:32:29PM +0100, Andreas Cadhalpun wrote:
> >> This fixes a heap-buffer-overflow in ff_er_frame_end when decoding mss2
> >> with
> >> coded_width/coded_height larger than width/height.
> >>
> >> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
> >> ---
> >> libavcodec/mpegpicture.c | 12 ++++++------
> >> 1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
> >> index 6748fc2..70b4d3c 100644
> >> --- a/libavcodec/mpegpicture.c
> >> +++ b/libavcodec/mpegpicture.c
> >> @@ -108,15 +108,15 @@ static int alloc_frame_buffer(AVCodecContext *avctx,
> >> Picture *pic,
> >> avctx->codec_id != AV_CODEC_ID_VC1IMAGE &&
> >> avctx->codec_id != AV_CODEC_ID_MSS2) {
> >> if (edges_needed) {
> >> - pic->f->width = avctx->width + 2 * EDGE_WIDTH;
> >> - pic->f->height = avctx->height + 2 * EDGE_WIDTH;
> >> + pic->f->width = avctx->coded_width + 2 * EDGE_WIDTH;
> >> + pic->f->height = avctx->coded_height + 2 * EDGE_WIDTH;
> >> }
> >>
> >> r = ff_thread_get_buffer(avctx, &pic->tf,
> >> pic->reference ? AV_GET_BUFFER_FLAG_REF
> >> : 0);
> >> } else {
> >> - pic->f->width = avctx->width;
> >> - pic->f->height = avctx->height;
> >> + pic->f->width = avctx->coded_width;
> >> + pic->f->height = avctx->coded_height;
> >> pic->f->format = avctx->pix_fmt;
> >> r = avcodec_default_get_buffer2(avctx, pic->f, 0);
> >> }
> >> @@ -135,8 +135,8 @@ static int alloc_frame_buffer(AVCodecContext *avctx,
> >> Picture *pic,
> >> (EDGE_WIDTH >> (i ? chroma_x_shift : 0));
> >> pic->f->data[i] += offset;
> >> }
> >> - pic->f->width = avctx->width;
> >> - pic->f->height = avctx->height;
> >> + pic->f->width = avctx->coded_width;
> >> + pic->f->height = avctx->coded_height;
> >> }
> >
> > why would the error concealment code need a differently sized output
> > than the decoder itself ?
>
> This code is rather convoluted, so it's not quite obvious, however the
> needed buffer size for the error concealment is determined by the
> MpegEncContext.width/height, which is set via:
> mss2_decode_init
> |-> wmv9_init
> |-> ff_msmpeg4_decode_init
> |-> ff_h263_decode_init
> |-> ff_mpv_decode_init:
> s->width = avctx->coded_width;
> s->height = avctx->coded_height;
>
> Thus the buffer needs the same size.Is it correct that your cases uses decode_wmv9() -> vc1_decode_i_blocks() ? these decode a rectangele up to end_mb_y, end_mb_x does this mismatch with what later code accesses ? would using end_mb_* in the EC code fix this ? (or disabling EC if they mismatch) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Does the universe only have a finite lifespan? No, its going to go on forever, its just that you wont like living in it. -- Hiranya Peiri
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
