[FFmpeg-devel] [PATCH] pnmdec: make sure v is capped by maxval

Michael Niedermayer michael at niedermayer.cc
Wed Nov 9 22:55:46 EET 2016


On Wed, Nov 09, 2016 at 09:05:17PM +0100, Andreas Cadhalpun wrote:
> On 09.11.2016 11:10, Michael Niedermayer wrote:
> > On Wed, Nov 09, 2016 at 01:11:29AM +0100, Andreas Cadhalpun wrote:
> >> Otherwise put_bits can be called with a value that doesn't fit in the
> >> sample_len, causing an assertion failure.
> >> ---
> >>  libavcodec/pnmdec.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
> >> index ca97cc3..0381ea6 100644
> >> --- a/libavcodec/pnmdec.c
> >> +++ b/libavcodec/pnmdec.c
> >> @@ -145,6 +145,10 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data,
> >>                          /* read a sequence of digits */
> >>                          do {
> >>                              v = 10*v + c;
> >> +                             if (v > s->maxval) {
> >> +                                av_log(avctx, AV_LOG_ERROR, "value %d larger than maxval %d\n", v, s->maxval);
> >> +                                return AVERROR_INVALIDDATA;
> >> +                            }
> > 
> > indention is a bit noisy
> 
> Fixed.
> 
> > i think it can overflow if maxval is large,
> 
> I've added an explicit check for v < 0, which should catch that.
> 
> > it would be faster to check outside the loop
> 
> However, such a check could pass if v overflowed so much that it's
> in the valid range again, so I'd rather not do that.
> 
> Updated patch is attached.
> 
> Best regards,
> Andreas
> 

>  pnmdec.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 9b84227c054610b73977e3acde32734a47e46c0c  0001-pnmdec-make-sure-v-is-capped-by-maxval.patch
> From 7e9dcbde04ad95fc93ac2f0e91d734c8187c8d2b Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Wed, 9 Nov 2016 01:09:35 +0100
> Subject: [PATCH] pnmdec: make sure v is capped by maxval
> 
> Otherwise put_bits can be called with a value that doesn't fit in the
> sample_len, causing an assertion failure.
> ---
>  libavcodec/pnmdec.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
> index ca97cc3..8961310 100644
> --- a/libavcodec/pnmdec.c
> +++ b/libavcodec/pnmdec.c
> @@ -145,6 +145,10 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data,
>                          /* read a sequence of digits */
>                          do {
>                              v = 10*v + c;
> +                            if (v < 0 || v > s->maxval) {

v < 0 implies v is signed
if 10*v overflows you have undefined behavior

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161109/89a15b6d/attachment.sig>


More information about the ffmpeg-devel mailing list