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

[Michael Niedermayer]

On Sun, May 08, 2011 at 01:04:10AM +0200, Stefano Sabatini wrote:
> 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

>  pnmdec.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 2dc846457256392b9c83b3ccc0ad4ade7676ab4a  0001-pnmdec-prevent-out-of-buffer-reads-in-pnm_decode_fra.patch
> From dd5e05dc9b4ae8c03e9a907f243ceb5de16234f4 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Sat, 7 May 2011 10:52:31 +0200
> Subject: [PATCH] pnmdec: prevent out-of-buffer reads in pnm_decode_frame()
> 
> ---
>  libavcodec/pnmdec.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
> index 6237e9a..147004b 100644
> --- a/libavcodec/pnmdec.c
> +++ b/libavcodec/pnmdec.c
> @@ -107,7 +107,7 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data,
>                      do{
>                          v= 10*v + c;
>                          c= (*s->bytestream++) - '0';
> -                    }while(c <= 9);
> +                    } while (c <= 9 && s->bytestream < s->bytestream_end);
>                      put_bits(&pb, sample_len, (((1<<sample_len)-1)*v + (s->maxval>>1))/s->maxval);

the buffer should be 0 padded so this should not be needed


>                  }
>                  flush_put_bits(&pb);
> -- 
> 1.7.2.3
> 

>  pnmdec.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> e9919269aa8a2bc136472dc2f038b895efe15295  0002-pnmdec-add-support-for-mono-images-with-non-space-se.patch
> From c3ff906eb20dc8ebe5f1d07e60c3fb5d8e5d325e Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Sun, 8 May 2011 00:47:14 +0200
> Subject: [PATCH] pnmdec: add support for mono images with non-space-separated pixel digits
> 
> 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 overflows.
> 
> 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 |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)

LGTM, thx


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct awnser.
-------------- 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/20110508/b8721159/attachment.asc>