On Tue, Feb 28, 2017 at 10:19:05PM -0500, Ronald S. Bultje wrote: > Hi Michael, > > On Tue, Feb 28, 2017 at 11:29 AM, Michael Niedermayer < > mich...@niedermayer.cc> wrote: > > > On Tue, Feb 28, 2017 at 07:44:12AM -0500, Ronald S. Bultje wrote: > > > Hi, > > > > > > On Mon, Feb 27, 2017 at 10:28 PM, Michael Niedermayer < > > > mich...@niedermayer.cc> wrote: > > > > > > > Fixes: 686/clusterfuzz-testcase-5853946876788736 > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss- > > > > fuzz/tree/master/targets/ffmpeg > > > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > > > --- > > > > libavcodec/vp8.c | 20 ++++++++++++++------ > > > > libavcodec/vp8.h | 2 +- > > > > 2 files changed, 15 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c > > > > index c1c3eb7072..cc158528ef 100644 > > > > --- a/libavcodec/vp8.c > > > > +++ b/libavcodec/vp8.c > > > > @@ -2275,7 +2275,7 @@ static void vp8_decode_mv_mb_modes( > > AVCodecContext > > > > *avctx, VP8Frame *cur_frame, > > > > #define update_pos(td, mb_y, mb_x) while(0) > > > > #endif > > > > > > > > -static av_always_inline void decode_mb_row_no_filter(AVCodecContext > > > > *avctx, void *tdata, > > > > +static av_always_inline int decode_mb_row_no_filter(AVCodecContext > > > > *avctx, void *tdata, > > > > int jobnr, int threadnr, int > > > > is_vp7) > > > > { > > > > VP8Context *s = avctx->priv_data; > > > > @@ -2291,6 +2291,10 @@ static av_always_inline void > > > > decode_mb_row_no_filter(AVCodecContext *avctx, void > > > > curframe->tf.f->data[1] + 8 * mb_y * s->uvlinesize, > > > > curframe->tf.f->data[2] + 8 * mb_y * s->uvlinesize > > > > }; > > > > + > > > > + if (c->end <= c->buffer && c->bits >= 0) > > > > + return AVERROR_INVALIDDATA; > > > > > > > > > From vp56.h: > > > > > > if(bits >= 0 && c->buffer < c->end) { > > > code_word |= bytestream_get_be16(&c->buffer) << bits; > > > bits -= 16; > > > } > > > > > > So this looks supicious, c->end should never be more than 1 byte beyond > > > c->buffer (which is padded by AV_INPUT_BUFFER_PADDING_SIZE). What is the > > > real issue? > > > > The real issue is that the code runs with blindfolds and hands tied > > behind its back, so it doesnt know when its running toward a wall > > and in fact it doesnt stop running when it hits the wall not even > > once it starved, its even running in its grave. > > > > You can feed the code with approximately a header with huge resolution > > and 0 bytes of actual image and it wont notice, and after a really long > > time it still wont notice it is decoding a image out of a non existing > > bitstream. > > > > That check checks if the end of the bitstream is reached and returns > > an error in that case. That way the test sample finished in 10sec > > instead of not finishing within the time i waited > > > Aaaaah, that explains things. > > The commit message uses "fuzz" and "fix" in the same sentence construction, > suggesting a security issue. You're now telling me there is in fact no > security issue. That suggests the commit message is wrong. > > I'm fine with the patch as is, but change the commit message. I would > suggest still referring to the filename, but avoiding the word "fix" since > it doesn't fix anything - since there is no bug. And then explain (in the
ive added "Timeout" after fix as thats what is "fixed", should avoid the ambiguity > commit message) that this shortcuts (i.e. speeds up) the error and > return-to-user when decoding a truncated frame. No error message is > necessary, it'll be something silly like "truncated bitstream" which > clobbers my terminal when I'm fuzzing and is otherwise virtually useless. added the suggeted text applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel