| Message ID | 20230921180912.10733-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/6] avcodec/osq: Check that pkt_offset does not exceed pkt size | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Thu, Sep 21, 2023 at 8:09 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > Fixes: out of array access > Fixes: > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6227491892887552 > Fixes: > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6268561729126400 > Fixes: > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6414805046788096 > Fixes: > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6538151088488448 > Fixes: > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6608131540779008 > > 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 <michael@niedermayer.cc> > --- > libavcodec/osq.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/osq.c b/libavcodec/osq.c > index e7f11691d2e..bcc75fef6fc 100644 > --- a/libavcodec/osq.c > +++ b/libavcodec/osq.c > @@ -408,6 +408,9 @@ static int osq_receive_frame(AVCodecContext *avctx, > AVFrame *frame) > GetBitContext *gb = &s->gb; > int ret, n; > > + if (s->pkt_offset > s->pkt->size) > + s->pkt_offset = 0; > This is more hack than real fix. Can you provide input file? > + > while (s->bitstream_size < s->max_framesize) { > int size; > > -- > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Thu, Sep 21, 2023 at 08:14:31PM +0200, Paul B Mahol wrote: > On Thu, Sep 21, 2023 at 8:09 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > Fixes: out of array access > > Fixes: > > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6227491892887552 > > Fixes: > > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6268561729126400 > > Fixes: > > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6414805046788096 > > Fixes: > > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6538151088488448 > > Fixes: > > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6608131540779008 > > > > 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 <michael@niedermayer.cc> > > --- > > libavcodec/osq.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/libavcodec/osq.c b/libavcodec/osq.c > > index e7f11691d2e..bcc75fef6fc 100644 > > --- a/libavcodec/osq.c > > +++ b/libavcodec/osq.c > > @@ -408,6 +408,9 @@ static int osq_receive_frame(AVCodecContext *avctx, > > AVFrame *frame) > > GetBitContext *gb = &s->gb; > > int ret, n; > > > > + if (s->pkt_offset > s->pkt->size) > > + s->pkt_offset = 0; > > > > This is more hack than real fix. why ? pkt->size is reset outside the codec, so either it needs to be checked on codec entry or the codec should not use internal->in_pkt and expect its size to be conserved or implement flush() or something ff_decode_flush_buffers() for example will clear teh packet if you prefer i can implement flush() and reset pkt_offset in it that probably would achieve teh same just say if you prefer that ? thx [...]
On 9/22/23, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Thu, Sep 21, 2023 at 08:14:31PM +0200, Paul B Mahol wrote: >> On Thu, Sep 21, 2023 at 8:09 PM Michael Niedermayer >> <michael@niedermayer.cc> >> wrote: >> >> > Fixes: out of array access >> > Fixes: >> > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6227491892887552 >> > Fixes: >> > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6268561729126400 >> > Fixes: >> > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6414805046788096 >> > Fixes: >> > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6538151088488448 >> > Fixes: >> > 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6608131540779008 >> > >> > 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 <michael@niedermayer.cc> >> > --- >> > libavcodec/osq.c | 3 +++ >> > 1 file changed, 3 insertions(+) >> > >> > diff --git a/libavcodec/osq.c b/libavcodec/osq.c >> > index e7f11691d2e..bcc75fef6fc 100644 >> > --- a/libavcodec/osq.c >> > +++ b/libavcodec/osq.c >> > @@ -408,6 +408,9 @@ static int osq_receive_frame(AVCodecContext *avctx, >> > AVFrame *frame) >> > GetBitContext *gb = &s->gb; >> > int ret, n; >> > >> > + if (s->pkt_offset > s->pkt->size) >> > + s->pkt_offset = 0; >> > >> >> This is more hack than real fix. > > why ? > > pkt->size is reset outside the codec, so either it needs to be > checked on codec entry or the codec should not use > internal->in_pkt and expect its size to be conserved > or implement flush() or something > > ff_decode_flush_buffers() for example will clear teh packet > > if you prefer i can implement flush() and reset pkt_offset in it > that probably would achieve teh same > just say if you prefer that ? Yup, that is much cleaner, go ahead with that solution with flush(). I forgot about flush() case completely. > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > When the tyrant has disposed of foreign enemies by conquest or treaty, and > there is nothing more to fear from them, then he is always stirring up > some war or other, in order that the people may require a leader. -- Plato >
diff --git a/libavcodec/osq.c b/libavcodec/osq.c index e7f11691d2e..bcc75fef6fc 100644 --- a/libavcodec/osq.c +++ b/libavcodec/osq.c @@ -408,6 +408,9 @@ static int osq_receive_frame(AVCodecContext *avctx, AVFrame *frame) GetBitContext *gb = &s->gb; int ret, n; + if (s->pkt_offset > s->pkt->size) + s->pkt_offset = 0; + while (s->bitstream_size < s->max_framesize) { int size;
Fixes: out of array access Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6227491892887552 Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6268561729126400 Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6414805046788096 Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6538151088488448 Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6608131540779008 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/osq.c | 3 +++ 1 file changed, 3 insertions(+)