[FFmpeg-devel] [PATCH] avcodec/vc1_block: Fix mqaunt check for negative values

[Michael Niedermayer]

On Fri, Jun 29, 2018 at 12:01:42PM +0200, Jerome Borsboom wrote:
> > Fixes: out of array access
> > Fixes: ffmpeg_bof_4.avi
> > Fixes: ffmpeg_bof_5.avi
> > Fixes: ffmpeg_bof_6.avi
> > 
> > Found-by: Thuan Pham, Marcel Böhme, Andrew Santosa and Alexandru Razvan Caciulescu with AFLSmart
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/vc1_block.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/vc1_block.c b/libavcodec/vc1_block.c
> > index 5427de4ec6..74d5e27803 100644
> > --- a/libavcodec/vc1_block.c
> > +++ b/libavcodec/vc1_block.c
> > @@ -204,7 +204,7 @@ static void vc1_put_blocks_clamped(VC1Context *v, int put_signed)
> >          if ((edges&8) &&                                       \
> >              s->mb_y == ((s->mb_height >> v->field_mode) - 1))  \
> >              mquant = -v->altpq;                                \
> > -        if (!mquant || mquant > 31) {                          \
> > +        if (!mquant || mquant > 31 || mquant < -31) {                          \
> >              av_log(v->s.avctx, AV_LOG_ERROR,                   \
> >                     "Overriding invalid mquant %d\n", mquant);  \
> >              mquant = 1;                                        \
> > -- 
> > 2.18.0
> 
> LGTM

will apply


> 
> However, we could consider to use saturation for invalid mquant values.
> 
> Something like:
> 
> mquant = mquant ? av_clip(mquant, -31, 31) : 1;

yes, it probably could be slightly better in some rare cases where
the decoder isnt totally confused and only produces random giberish anymore


> 
> 
> I would prefer to catch illegal values at the earliest occasion. Illegal
> v->pq or v->altpq should be catched earlier, in my view. A the current
> implementation is technically correct, this can wait for another time.

yes, i agree

thx

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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180629/d48c5a97/attachment.sig>