On Tue, Aug 07, 2018 at 09:07:11PM -0400, Ronald S. Bultje wrote: > Hi, > > On Tue, Aug 7, 2018 at 7:15 PM, Michael Niedermayer <mich...@niedermayer.cc> > wrote: > > > On Mon, Aug 06, 2018 at 09:50:57PM -0400, Ronald S. Bultje wrote: > > > Hi, > > > > > > On Mon, Aug 6, 2018 at 3:00 PM, Michael Niedermayer > > <mich...@niedermayer.cc> > > > wrote: > > > > > > > On Tue, Aug 07, 2018 at 01:05:51AM +0800, Ronald S. Bultje wrote: > > > > > Hi, > > > > > > > > > > On Sun, Aug 5, 2018, 9:17 AM Michael Niedermayer > > <mich...@niedermayer.cc > > > > > > > > > > wrote: > > > > > > > > > > > Fixes: Timeout > > > > > > Fixes: > > > > > > 9330/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ > > > > VP9_fuzzer-5707345857347584 > > > > > > > > > > > > Found-by: continuous fuzzing process > > > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > > > Signed-off-by > > > > > > <https://github.com/google/oss-fuzz/tree/master/projects/ > > > > ffmpegSigned-off-by>: > > > > > > Michael Niedermayer <mich...@niedermayer.cc> > > > > > > --- > > > > > > libavcodec/vp9.c | 3 +++ > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c > > > > > > index b1178c9c0c..4ca51ec108 100644 > > > > > > --- a/libavcodec/vp9.c > > > > > > +++ b/libavcodec/vp9.c > > > > > > @@ -1302,6 +1302,9 @@ static int decode_tiles(AVCodecContext > > *avctx, > > > > > > memset(lflvl_ptr->mask, 0, > > > > > > sizeof(lflvl_ptr->mask)); > > > > > > } > > > > > > > > > > > > + if (td->c->end <= td->c->buffer && > > td->c->bits >= > > > > 0) { > > > > > > + return AVERROR_INVALIDDATA; > > > > > > + } > > > > > > if (s->pass == 2) { > > > > > > decode_sb_mem(td, row, col, lflvl_ptr, > > > > > > yoff2, uvoff2, BL_64X64); > > > > > > > > > > > > > > > > I don't think this matches spec. Implementations could use this to > > store > > > > > auxiliary data. > > > > > > > > This checks, or rather is intended to check for a premature end of the > > > > input. > > > > Am i missing something? because a premature end of input seems not to > > lead > > > > to more (auxiliary or other) data in the input. > > > > > > > > > Hm, I misread it, sorry about that, my bad. Please ignore my first > > review. > > > > > > > > Is end == buffer && bits == 0 something that can happen on valid input if > > > things just align properly? Otherwise looks OK. > > > > The same condition is used in vp5/6/8. > > I think this condition only occurs if the input ends. The part i do not > > know > > is if such premature ends might be a "valid compression" > > > > Either way, if this check misbehaves for a valid file then it should be > > reverted/removed unless its improv-able and improved. > >
> <sarcasm> Yes, that's how production grade software works. </sarcasm> yes ;) but seriously, It only needs a single user hitting a bug and reporting it for us to know its a issue and revert. This is not in a release just git master. Its my oppinion that this is wiser than to never attempt to fix the issue. Which ultimatly is the alternative. That is unless you know that this is correct or incorrect for all cases. Maybe we have very differnt views of how to do software development. My goal is to minimize the amount of issues in the code per developer time spend. Time is a limited resource. Doing anything else simply leads to worse code. If i spend a week testing code that i know is 99.8% right to make it 99.9% right just to save a single user from once hitting and reporting a non security bug. That is a loss as i could have spend that week fixing many other issues. Its even worse if the 99.9% still hits one user and gets reverted, in this case there is no improvment in user experience at all for the time spend. In either the 99.8 and 99.9% cases one user ultimatly hits a file that breaks and teh change gets reverted. but in one case we would have spend alot more time. So yes, it is my philosophy to use our users as beta testers. Not as a default no absolutely not, but in the rare cases where the extra work we cause our users is comparably tiny compared to how much work we would have to do to avoid it. Or if we can never really match user testing anyway And we also did and do this all over the place. For example, if we do not know how to handle some case and have no sample, we print a message and ask the user for a sample. This is comparable to the case here. "Is there a sample out there for which this check misbehaves" vs. "Is there a file out there that uses a special feature" Either is a question about global file existence and search > Can > you just make it not error out on the end == buffer && bits == 0 condition? > Or does that somehow not fix your timeout? If the code continues processing "nothing" and does alot of computations on it, it will need a long time to complete that. Thats the issue. Its like a network router deciding to spend an hour processing a 100byte packet that says its 4gb large in a header without there anywhere being 4gb of input data. simple denial of service. Thanks [...] -- 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
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
