On 26.02.2016, at 11:26, wm4 <nfx...@googlemail.com> wrote: > On Thu, 25 Feb 2016 22:39:51 +0100 > Reimar Döffinger <reimar.doeffin...@gmx.de> wrote: > >> On Thu, Feb 25, 2016 at 09:25:08PM +0100, wm4 wrote: >>> On Thu, 25 Feb 2016 21:06:46 +0100 >>> Reimar Döffinger <reimar.doeffin...@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.doeffin...@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. >> >> I am not sure there is a fixed requirement for the relation >> between coded_height and height that necessarily applies >> to all codecs. > > Well, coded_width is supposed to be the actual frame size, while width > applies cropping. So width should always be <= coded_width. For codecs > where this should not hold up for some reason, coded_width is obviously > inappropriate to use in the first place.
While that's the for possibly all codecs, I meant I couldn't see clear documentation that expanding via height > coded_height would be impossible if some codec had use for it. Either way I'll push it soonish if nobody has comments on the patch itself... _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
