[FFmpeg-devel] [PATCH] vf_unsharp: extend/improve feedback for validity checks

[Michael Niedermayer]

On Mon, Feb 06, 2012 at 03:41:41PM +0100, Stefano Sabatini wrote:
> On date Sunday 2011-08-14 15:03:56 +0200, Michael Niedermayer encoded:
> > On Sun, Aug 14, 2011 at 02:38:52PM +0200, Michael Niedermayer wrote:
> > > On Sat, Aug 13, 2011 at 01:11:49AM +0200, Stefano Sabatini wrote:
> > > > Abort for invalid too big values, and exactly state why the input
> > > > value is invalid.
> > > > 
> > > > In particular, avoid out-of-buffer access with too big values.
> > > > ---
> > > >  libavfilter/vf_unsharp.c |   20 ++++++++++++++------
> > > >  1 files changed, 14 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/libavfilter/vf_unsharp.c b/libavfilter/vf_unsharp.c
> > > > index e41e76f..9c85e5d 100644
> > > > --- a/libavfilter/vf_unsharp.c
> > > > +++ b/libavfilter/vf_unsharp.c
> > > > @@ -135,19 +135,27 @@ static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> > > >      UnsharpContext *unsharp = ctx->priv;
> > > >      int lmsize_x = 5, cmsize_x = 0;
> > > >      int lmsize_y = 5, cmsize_y = 0;
> > > > +    int val;
> > > >      double lamount = 1.0f, camount = 0.0f;
> > > >  
> > > >      if (args)
> > > >          sscanf(args, "%d:%d:%lf:%d:%d:%lf", &lmsize_x, &lmsize_y, &lamount,
> > > >                                              &cmsize_x, &cmsize_y, &camount);
> > > >  
> > > > -    if ((lamount && (lmsize_x < 2 || lmsize_y < 2)) ||
> > > > -        (camount && (cmsize_x < 2 || cmsize_y < 2))) {
> > > > -        av_log(ctx, AV_LOG_ERROR,
> > > > -               "Invalid value <2 for lmsize_x:%d or lmsize_y:%d or cmsize_x:%d or cmsize_y:%d\n",
> > > > -               lmsize_x, lmsize_y, cmsize_x, cmsize_y);
> > > > -        return AVERROR(EINVAL);
> > > > +#define CHECK_SIZE(lc, xy, lc_str)                                      \
> > > > +    val = lc##msize_##xy;                                               \
> > > > +    if (val < MATRIX_MIN_LINE_SIZE || val > MATRIX_MAX_LINE_SIZE) {     \
> > > > +        av_log(ctx, AV_LOG_ERROR,                                       \
> > > > +               "Invalid value '%d' for %s %s size, "                    \
> > > > +               "must be >= %d and <= %d\n",                             \
> > > > +               val, #lc_str, #xy,                                       \
> > > > +               MATRIX_MIN_LINE_SIZE, MATRIX_MAX_LINE_SIZE);             \
> > > 
> > > The values look wrong, the arrays are not 2d to begin with in the way
> > > the port assumes so checking against a x of a x*x array isnt likely
> > > correct.
> > > 
> > 
> > > The checks from the original libmpocodecs code should be used:
> > >      uint32_t *SC[MAX_MATRIX_SIZE-1];
> > >      ...
> > >      fp->msizeX = 1 | av_clip(fp->msizeX, MIN_MATRIX_SIZE, MAX_MATRIX_SIZE);
> > >      fp->msizeY = 1 | av_clip(fp->msizeY, MIN_MATRIX_SIZE, MAX_MATRIX_SIZE);
> > 
> > to calrify, iam not suggesting that this be used litterally but the
> > max/min values of it, which look more correct to me
> 
> Updated, perform the same checks of the original MP filter.
> -- 
> FFmpeg = Fostering and Forgiving Monstrous Powerful Eretic Gnome

>  vf_unsharp.c |   20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 907f8c2fc4cce4585477ebf98f97dcaa27560771  0002-vf_unsharp-extend-validity-checks-and-improve-feedba.patch
> From 740645238b359327c82ab4931c8eded92fbadb60 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Sat, 13 Aug 2011 01:08:48 +0200
> Subject: [PATCH] vf_unsharp: extend validity checks, and improve feedback
> 
> Abort for invalid too big input values, and clearly state the failure
> reason.
> 
> In particular, avoid out-of-buffer access and crash with too big values.

LGTM



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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120312/47936ce5/attachment.asc>