| Message ID | CAB0OVGrQtQpbKRuXSnaif+DgmiTfsO03uYy94z4GZAwzTug63w@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 9/5/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-05 9:21 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>: >> On Sun, Sep 04, 2016 at 08:58:44PM +0200, Carl Eugen Hoyos wrote: > >>> @@ -159,6 +163,8 @@ static int pnm_decode_frame(AVCodecContext *avctx, >>> void *data, >>> } >>> }else{ >>> for (i = 0; i < avctx->height; i++) { >>> + if (s->bytestream + n > s->bytestream_end) >>> + continue; >> >> having a pointer point outside of 0..array length is undefined >> behaviour (and can overflow in principle) > > > New patch attached. It seems this patch disables check for all cases when experimental is enabled, but check for overflow in only one case.
2016-09-05 10:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > On 9/5/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> New patch attached. > > It seems this patch disables check for all cases when experimental is enabled, > but check for overflow in only one case. I am not sure I understand: Do you mean I missed a case where an overflow is now (after the patch) possible (but wasn't before) or do you mean there are formats after the patch that allow truncation and formats that do not allow it? Carl Eugen
2016-09-05 11:12 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > 2016-09-05 10:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >> On 9/5/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >>> New patch attached. >> >> It seems this patch disables check for all cases when experimental is enabled, >> but check for overflow in only one case. > > I am not sure I understand: > Do you mean I missed a case where an overflow is now (after the patch) > possible (but wasn't before) or do you mean there are formats after the > patch that allow truncation and formats that do not allow it? Ping. Carl Eugen
On Tue, Oct 11, 2016 at 10:06:54AM +0200, Carl Eugen Hoyos wrote: > 2016-09-05 11:12 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > > 2016-09-05 10:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > >> On 9/5/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >>> New patch attached. > >> > >> It seems this patch disables check for all cases when experimental is enabled, > >> but check for overflow in only one case. > > > > I am not sure I understand: > > Do you mean I missed a case where an overflow is now (after the patch) > > possible (but wasn't before) or do you mean there are formats after the > > patch that allow truncation and formats that do not allow it? > > Ping. i didnt look at the code but from the diff it seems what was meant was that bytestream + n could point outside the array that is indeed (suprising to many) undefined, you dont need to do bytestream[n] [...]
On Tue, Oct 11, 2016 at 11:16:48AM +0200, Michael Niedermayer wrote: > On Tue, Oct 11, 2016 at 10:06:54AM +0200, Carl Eugen Hoyos wrote: > > 2016-09-05 11:12 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > > > 2016-09-05 10:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > > >> On 9/5/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > >>> New patch attached. > > >> > > >> It seems this patch disables check for all cases when experimental is enabled, > > >> but check for overflow in only one case. > > > > > > I am not sure I understand: > > > Do you mean I missed a case where an overflow is now (after the patch) > > > possible (but wasn't before) or do you mean there are formats after the > > > patch that allow truncation and formats that do not allow it? > > > > Ping. > > i didnt look at the code but from the diff it seems what was > meant was that bytestream + n could point outside the array > that is indeed (suprising to many) undefined, you dont need to do > bytestream[n] i just stumbled across this again the correct way to check for the end (overflow wise) is if (n > s->bytestream_end - s->bytestream) ... also ptr[] should be memset (probably to 0) when there is no more input [...]
From af00c56b38b28e07bbba46031472da41300a8cf1 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <cehoyos@ag.or.at> Date: Sun, 4 Sep 2016 20:52:28 +0200 Subject: [PATCH] lavc/pnmdec: Do not fail by default for truncated pbm files. Fixes ticket #5795. --- libavcodec/pnmdec.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c index d4261a4..0b7a0f6 100644 --- a/libavcodec/pnmdec.c +++ b/libavcodec/pnmdec.c @@ -124,8 +124,12 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data, do_read: ptr = p->data[0]; linesize = p->linesize[0]; - if (n * avctx->height > s->bytestream_end - s->bytestream) - return AVERROR_INVALIDDATA; + if (n * avctx->height > s->bytestream_end - s->bytestream) { + av_log(avctx, AV_LOG_ERROR, + "Invalid truncated file\n"); + if (avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT) + return AVERROR_INVALIDDATA; + } if(s->type < 4 || (is_mono && s->type==7)){ for (i=0; i<avctx->height; i++) { PutBitContext pb; @@ -159,6 +163,8 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data, } }else{ for (i = 0; i < avctx->height; i++) { + if (s->bytestream > s->bytestream_end - n) + continue; if (!upgrade) samplecpy(ptr, s->bytestream, n, s->maxval); else if (upgrade == 1) { -- 1.7.10.4