[FFmpeg-devel] [libav-devel] [PATCH] vp8: check for too large dimensions

Ronald S. Bultje rsbultje at gmail.com
Sun Jun 7 22:27:42 CEST 2015


Hi,

On Sun, Jun 7, 2015 at 2:35 PM, Andreas Cadhalpun <
andreas.cadhalpun at googlemail.com> wrote:

> Hi Ronald,
>
> On 07.06.2015 19:37, Ronald S. Bultje wrote:
> > On Sun, Jun 7, 2015 at 12:44 PM, Andreas Cadhalpun <
> > andreas.cadhalpun at googlemail.com> wrote:
> >> On 07.06.2015 17:52, Ronald S. Bultje wrote:
> >>> We can't simply claim that 8k is not a valid dimension, that would
> make us
> >>> a vp8-incompatible decoder if vp8 stream dimensions are allowed to be
> >>> 8k.
> >>> This could particularly happen in images (webp).
> >>
> >> I'd change the error to AVERROR_PATCHWELCOME as Michael suggested, so
> feel
> >> free to fix this limitation. ;)
> >
> >
> > No, no, not so easy. For example, keyframes (like webp) would decode just
> > fine even thougb mvmin/max is broken, so we're causing a regression here
> of
> > something that worked fine before.
>
> Hmm, so maybe the check could be moved to decode_mb_row_no_filter?
>
> > Is this an actual bug? What is the behaviour that you're trying to fix?
> Is
> > this some overflow noticed on some generated/fuzzed bitstream with
> > -fsanitize=integer? Or are we decoding a sample differently from libvpx?
> Or
> > something else?
>
> The overflow of mv_max can cause it to be smaller than mv_min, which causes
> av_clip to abort for ASSERT_LEVEL >= 2.


So what happens when you change mv_max/min to be integers (instead of
int16_t) without further touching VP56mv, e.g. by making mv_max/min a
VP8intminmaxmv (a newly created type just for this purpose, using int
instead of int16_t)?

Ronald


More information about the ffmpeg-devel mailing list