Re: [FFmpeg-devel] [PATCH] avformat/ape: Error out in case of EOF in the header

2020-07-25 Thread Michael Niedermayer 
On Sat, Jul 25, 2020 at 06:10:44AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: OOM
> > Fixes: 
> > 24375/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6216862443241472
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/ape.c | 8 +---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavformat/ape.c b/libavformat/ape.c
> > index 39a584aa98..e966063ad7 100644
> > --- a/libavformat/ape.c
> > +++ b/libavformat/ape.c
> > @@ -253,7 +253,7 @@ static int ape_read_header(AVFormatContext * s)
> >  avio_skip(pb, ape->wavheaderlength);
> >  }
> >  
> > -if(!ape->totalframes){
> > +if(!ape->totalframes || pb->eof_reached){
> >  av_log(s, AV_LOG_ERROR, "No frames in the file!\n");
> >  return AVERROR(EINVAL);
> >  }
> > @@ -298,8 +298,10 @@ static int ape_read_header(AVFormatContext * s)
> >  for (i = 0; i < ape->totalframes && !pb->eof_reached; i++)
> >  ape->bittable[i] = avio_r8(pb);
> >  }
> > -if (pb->eof_reached)
> > -av_log(s, AV_LOG_WARNING, "File truncated\n");
> > +if (pb->eof_reached) {
> > +av_log(s, AV_LOG_ERROR, "File truncated\n");
> > +return AVERROR_INVALIDDATA;
> > +}
> 
> The above will lead to a memleak as long as cleaning up on read_header
> failure is not automatic. Furthermore, it's the first of the checks that
> fixes the OOM scenario, not the second, isn't it?

yes, will apply with the leak issue fixed


> 
> - Andreas
> 
> PS: One could remove the allocations for seek- and bittable altogether;
> only ape_dumpinfo() needs to have the seektable, the bittable as well as
> the frames at the same time.

will post a patch doing that

thx

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

The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/ape: Error out in case of EOF in the header

2020-07-24 Thread Andreas Rheinhardt 
Michael Niedermayer:
> Fixes: OOM
> Fixes: 
> 24375/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6216862443241472
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/ape.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/ape.c b/libavformat/ape.c
> index 39a584aa98..e966063ad7 100644
> --- a/libavformat/ape.c
> +++ b/libavformat/ape.c
> @@ -253,7 +253,7 @@ static int ape_read_header(AVFormatContext * s)
>  avio_skip(pb, ape->wavheaderlength);
>  }
>  
> -if(!ape->totalframes){
> +if(!ape->totalframes || pb->eof_reached){
>  av_log(s, AV_LOG_ERROR, "No frames in the file!\n");
>  return AVERROR(EINVAL);
>  }
> @@ -298,8 +298,10 @@ static int ape_read_header(AVFormatContext * s)
>  for (i = 0; i < ape->totalframes && !pb->eof_reached; i++)
>  ape->bittable[i] = avio_r8(pb);
>  }
> -if (pb->eof_reached)
> -av_log(s, AV_LOG_WARNING, "File truncated\n");
> +if (pb->eof_reached) {
> +av_log(s, AV_LOG_ERROR, "File truncated\n");
> +return AVERROR_INVALIDDATA;
> +}

The above will lead to a memleak as long as cleaning up on read_header
failure is not automatic. Furthermore, it's the first of the checks that
fixes the OOM scenario, not the second, isn't it?

- Andreas

PS: One could remove the allocations for seek- and bittable altogether;
only ape_dumpinfo() needs to have the seektable, the bittable as well as
the frames at the same time.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/ape: Error out in case of EOF in the header

2020-07-24 Thread Michael Niedermayer 
Fixes: OOM
Fixes: 
24375/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6216862443241472

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavformat/ape.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libavformat/ape.c b/libavformat/ape.c
index 39a584aa98..e966063ad7 100644
--- a/libavformat/ape.c
+++ b/libavformat/ape.c
@@ -253,7 +253,7 @@ static int ape_read_header(AVFormatContext * s)
 avio_skip(pb, ape->wavheaderlength);
 }
 
-if(!ape->totalframes){
+if(!ape->totalframes || pb->eof_reached){
 av_log(s, AV_LOG_ERROR, "No frames in the file!\n");
 return AVERROR(EINVAL);
 }
@@ -298,8 +298,10 @@ static int ape_read_header(AVFormatContext * s)
 for (i = 0; i < ape->totalframes && !pb->eof_reached; i++)
 ape->bittable[i] = avio_r8(pb);
 }
-if (pb->eof_reached)
-av_log(s, AV_LOG_WARNING, "File truncated\n");
+if (pb->eof_reached) {
+av_log(s, AV_LOG_ERROR, "File truncated\n");
+return AVERROR_INVALIDDATA;
+}
 }
 
 ape->frames[0].pos = ape->firstframe;
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".