| Message ID | 6880ed2d-64e0-3a28-9f67-06d05cb68f14@googlemail.com |
|---|---|
| State | Superseded |
| Headers | show |
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@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 [...]
From 7e9dcbde04ad95fc93ac2f0e91d734c8187c8d2b Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun <Andreas.Cadhalpun@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) { + av_log(avctx, AV_LOG_ERROR, "value %d larger than maxval %d (or overflown)\n", v, s->maxval); + return AVERROR_INVALIDDATA; + } c = (*s->bytestream++) - '0'; } while (c <= 9); } -- 2.10.2