[FFmpeg-devel] Security issues?

[Reimar Döffinger]

On Wed, Sep 23, 2009 at 03:22:02PM +0200, Michael Niedermayer wrote:
> On Wed, Sep 23, 2009 at 03:15:06PM +0200, Reimar D?ffinger wrote:
> > On Wed, Sep 23, 2009 at 02:57:31PM +0200, Michael Niedermayer wrote:
> > > > Index: libavcodec/vorbis_dec.c
> > > > ===================================================================
> > > > --- libavcodec/vorbis_dec.c     (revision 19987)
> > > > +++ libavcodec/vorbis_dec.c     (working copy)
> > > > @@ -162,6 +162,13 @@
> > > >  #define BARK(x) \
> > > >      (13.1f*atan(0.00074f*(x))+2.24f*atan(1.85e-8f*(x)*(x))+1e-4f*(x))
> > > >  
> > > > +
> > > > +#define VALIDATE_INDEX(ctx, idx, limit, onerror) \
> > > 
> > > > +    if (idx >= limit) {\
> > > 
> > > the check should be an argument to the macro
> > > 
> > > 
> > > > +        av_log(ctx, AV_LOG_ERROR, "Index value %d out of range (0 - %d) for "#idx "\n", idx, limit);\
> > 
> > Uh, and what should be the message then? If you change the check, this
> > message no longer makes sense, 
> 
> the message could be an argument too

The message and the arguments then. And IMO then you don't really save
much.
For what guess, there are maybe 10 places where this macro can be
used unchanged and a few where you'd have to add a if (... != -1),
and I just think a macro for this special case alone would be justified
and making it more generic just re-adds some of the bloat it is supposed
to avoid...