[FFmpeg-devel] [PATCH 1/2] avcodec/snowdec: Check intra block dc differences.

[Ronald S. Bultje]

Hi,

On Wed, Nov 15, 2017 at 7:02 PM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> On Wed, Nov 15, 2017 at 03:26:42PM -0500, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Wed, Nov 15, 2017 at 3:17 PM, Michael Niedermayer
> <michael at niedermayer.cc
> > > wrote:
> >
> > > Fixes: Timeout
> > > Fixes: 3142/clusterfuzz-testcase-5007853163118592
> > >
> > > Found-by: continuous fuzzing process https://github.com/google/oss-
> > > fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > ---
> > >  libavcodec/snowdec.c | 19 +++++++++++++++----
> > >  1 file changed, 15 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/libavcodec/snowdec.c b/libavcodec/snowdec.c
> > > index 727e908fb5..77ffe7f594 100644
> > > --- a/libavcodec/snowdec.c
> > > +++ b/libavcodec/snowdec.c
> > > @@ -183,13 +183,24 @@ static int decode_q_branch(SnowContext *s, int
> > > level, int x, int y){
> > >          int my_context= av_log2(2*FFABS(left->my - top->my)) +
> > > 0*av_log2(2*FFABS(tr->my - top->my));
> > >
> > >          type= get_rac(&s->c, &s->block_state[1 + left->type +
> top->type])
> > > ? BLOCK_INTRA : 0;
> > > -
> > >          if(type){
> > > +            int ld, cbd, crd;
> > >              pred_mv(s, &mx, &my, 0, left, top, tr);
> > > -            l += get_symbol(&s->c, &s->block_state[32], 1);
> > > +            ld = get_symbol(&s->c, &s->block_state[32], 1);
> > > +            if (ld < -255 || ld > 255) {
> > > +                av_log(s->avctx, AV_LOG_DEBUG, "Invalid (Out of range)
> > > intra luma block DC difference %d\n", ld);
> > > +                return AVERROR_INVALIDDATA;
> > > +            }
> > > +            l += ld;
> > >              if (s->nb_planes > 2) {
> > > -                cb+= get_symbol(&s->c, &s->block_state[64], 1);
> > > -                cr+= get_symbol(&s->c, &s->block_state[96], 1);
> > > +                cbd = get_symbol(&s->c, &s->block_state[64], 1);
> > > +                crd = get_symbol(&s->c, &s->block_state[96], 1);
> > > +                if (cbd < -255 || cbd > 255 || crd < -255 || crd >
> 255) {
> > > +                    av_log(s->avctx, AV_LOG_DEBUG, "Invalid (Out of
> > > range) intra chroma block DC difference %d, %d\n", cbd, crd);
> > > +                    return AVERROR_INVALIDDATA;
> > > +                }
> >
> >
> > Please remove the error messages.
>
> We had this discussion multiple times already.
> I would prefer to keep an error message as its important in bug
> reporting and to maintain and debug this code which iam maintainer and
> author of.
>
> Some similar previous discussion for example:
> http://ffmpeg.org/pipermail/ffmpeg-devel/2017-September/216499.html
>
> To repeat from the thread above:
> "Iam happy to follow what the community prefers."
>
> It seems you dont want to poll the community
>
> Is your politly worded request meant litterally
> just as a suggestion (which i can ignore) ?
> or did you intend this to be a veto ?
> Which i would of course respect even though iam not sure you have veto
> power over maintainer and author.
>
> On top of that, this is part of a security fix for an issue that will
> be made (automatically) public soon.


So, commit it without the error message? I really don't see the issue. Is
the lack of error message the security issue?

Ronald