[FFmpeg-devel] [PATCH 1/2] avcodec/speexdec: Check frames_per_packet more completely

[Michael Niedermayer]

On Fri, Oct 15, 2021 at 07:30:20PM +0200, Paul B Mahol wrote:
> On Fri, Oct 15, 2021 at 7:26 PM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > On Fri, Oct 15, 2021 at 08:52:39AM +0200, Paul B Mahol wrote:
> > > On Fri, Oct 15, 2021 at 12:20 AM James Almer <jamrial at gmail.com> wrote:
> > >
> > > > On 10/14/2021 7:13 PM, Michael Niedermayer wrote:
> > > > > Fixes: signed integer overflow: 2105344 * 539033345 cannot be
> > > > represented in type 'int'
> > > > > Fixes: out of array write
> > > > > Fixes:
> > > >
> > 39956/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SPEEX_fuzzer-4766419250708480
> > > > >
> > > > > Found-by: continuous fuzzing process
> > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > > > ---
> > > > >   libavcodec/speexdec.c | 2 +-
> > > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/libavcodec/speexdec.c b/libavcodec/speexdec.c
> > > > > index 4c50f54f27b..05f084de2e5 100644
> > > > > --- a/libavcodec/speexdec.c
> > > > > +++ b/libavcodec/speexdec.c
> > > > > @@ -1428,7 +1428,7 @@ static int parse_speex_extradata(AVCodecContext
> > > > *avctx,
> > > > >           return AVERROR_INVALIDDATA;
> > > > >       s->vbr = bytestream_get_le32(&buf);
> > > > >       s->frames_per_packet = bytestream_get_le32(&buf);
> > > > > -    if (s->frames_per_packet <= 0)
> > > > > +    if (s->frames_per_packet <= 0 || s->frames_per_packet >=
> > INT32_MAX
> > > > / s->frame_size)
> > > >
> > > > s->frames_per_packet * s->frame_size is then multiplied by channel
> > count
> > > > (which can be at most 2) for the vector_fmul_scalar() call, so might as
> > > > well check for that here too.
> > > >
> > > > Alternatively, all floatdsp functions could be adapted to take size_t
> > > > buffer lengths.
> > > >
> > >
> > > Limit it to 32/64.
> >
> > you mean like this: (64 channels) ?
> >
> 
> I mean limit max frames per packet to 64.

that would give something like this: unless we can bound frame_size tighter

diff --git a/libavcodec/speexdec.c b/libavcodec/speexdec.c
index 4c50f54f27b..d240e8a20bf 100644
--- a/libavcodec/speexdec.c
+++ b/libavcodec/speexdec.c
@@ -1428,7 +1428,9 @@ static int parse_speex_extradata(AVCodecContext *avctx,
         return AVERROR_INVALIDDATA;
     s->vbr = bytestream_get_le32(&buf);
     s->frames_per_packet = bytestream_get_le32(&buf);
-    if (s->frames_per_packet <= 0)
+    if (s->frames_per_packet <= 0 ||
+        s->frames_per_packet > 64 ||
+        s->frames_per_packet >= INT32_MAX / s->nb_channels / s->frame_size)
         return AVERROR_INVALIDDATA;
     s->extra_headers = bytestream_get_le32(&buf);

 
[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If the United States is serious about tackling the national security threats 
related to an insecure 5G network, it needs to rethink the extent to which it
values corporate profits and government espionage over security.-Bruce Schneier
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20211015/99ab9a66/attachment.sig>