[FFmpeg-devel] [PATCH] pnmdec: add support for mono images with non-space-separated pixel digits

[Stefano Sabatini]

On date Saturday 2011-05-07 13:44:53 +0200, Michael Niedermayer encoded:
> On Sat, May 07, 2011 at 11:05:20AM +0200, Stefano Sabatini wrote:
> > When the file to decode contains a sequence of binary values like
> > "1101110...", decode_frame() was reading the sequence of digits like a
> > unique integer value, which was resulting in integer overflow and
> > out-of-buffer reads.
> > 
> > The change add support for parsing non-space-separated pixel digits
> > for mono formats, in particular fix decoding of file battrace.pbm, and
> > fix trac issue #154.
> > ---
> >  libavcodec/pnmdec.c |   14 ++++++++++----
> >  1 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
> > index 6237e9a..53e50af 100644
> > --- a/libavcodec/pnmdec.c
> > +++ b/libavcodec/pnmdec.c
> > @@ -104,10 +104,16 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data,
> >                          s->bytestream++;
> >                      if(s->bytestream >= s->bytestream_end)
> >                          return -1;
> > -                    do{
> > -                        v= 10*v + c;
> > -                        c= (*s->bytestream++) - '0';
> > -                    }while(c <= 9);
> > +                    while (s->bytestream < s->bytestream_end) {
> > +                        c = (*s->bytestream++) - '0';
> > +                        if (c > 9)
> > +                            break;
> > +                        v = 10*v + c;
> > +                        if ((avctx->pix_fmt == PIX_FMT_MONOWHITE ||
> > +                             avctx->pix_fmt == PIX_FMT_MONOBLACK) &&
> > +                            *s->bytestream - '0' <= 9)
> > +                            break;
> > +                    }
> 
> This is done per sample thus its speed critical and the if() should be
> outside the loop, something like
> if(...)
>     do{
>         v= 10*v + c;
>         c= (*s->bytestream++) - '0';
>     }while(c <= 9);
> else
>     v=(*s->bytestream++) - '0'
> 
> would be better but of course this code can be optimized alot more if
> someone wants

See attached, out-of-buffer read check added as a separate patch.
-- 
FFmpeg = Fantastic Frightening MultiPurpose Erudite Generator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-pnmdec-prevent-out-of-buffer-reads-in-pnm_decode_fra.patch
Type: text/x-diff
Size: 947 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110508/66aa5e56/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-pnmdec-add-support-for-mono-images-with-non-space-se.patch
Type: text/x-diff
Size: 2325 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110508/66aa5e56/attachment-0001.bin>