On Thu, Nov 24, 2016 at 05:45:38PM +0100, Michael Niedermayer wrote:
> 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) Note, for this sadly end_mb_* in MSS2 would need to be treated differently than other codecs as it has different semantics disabling EC on end_mb_ mismatch might be easier [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
