Re: [FFmpeg-devel] [PATCH 1/6] avcodec/osq: Check that pkt_offset does not exceed pkt size

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 1/6] avcodec/osq: Check that pkt_offset does not exceed pkt size
Date: Fri, 22 Sep 2023 18:47:48 +0200
Message-ID: <20230922164748.GY8640@pb2> (raw)
In-Reply-To: <CAPYw7P7hvzMp30GtvewAM_ns6+U-U9rwLOF2nTw+V4f2s3iHzg@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2221 bytes --]

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

[...]
-- 
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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".