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

[Michael Niedermayer]

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.


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171116/33c1de73/attachment.sig>