[FFmpeg-devel] [PATCH 3/3] avformat/mov: Check for EOF in mov_read_meta()

[Michael Niedermayer]

On Sun, Sep 15, 2019 at 04:35:30PM -0300, James Almer wrote:
> On 9/15/2019 4:34 PM, Michael Niedermayer wrote:
> > On Sat, Aug 31, 2019 at 10:36:32AM -0300, James Almer wrote:
> >> On 8/31/2019 5:47 AM, Michael Niedermayer wrote:
> >>> On Fri, Aug 30, 2019 at 08:57:29PM -0300, James Almer wrote:
> >>>> On 8/30/2019 8:25 PM, Michael Niedermayer wrote:
> >>>>> Fixes: Timeout (195sec -> 2ms)
> >>>>> Fixes: 16735/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5090676403863552
> >>>>>
> >>>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> >>>>> ---
> >>>>>  libavformat/mov.c | 5 ++++-
> >>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >>>>> index 675b915906..46c544b61f 100644
> >>>>> --- a/libavformat/mov.c
> >>>>> +++ b/libavformat/mov.c
> >>>>> @@ -4419,7 +4419,10 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>>>>  static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>>>>  {
> >>>>>      while (atom.size > 8) {
> >>>>> -        uint32_t tag = avio_rl32(pb);
> >>>>> +        uint32_t tag;
> >>>>> +        if (avio_feof(pb))
> >>>>> +            return AVERROR_EOF;
> >>>>> +        tag = avio_rl32(pb);
> >>>>>          atom.size -= 4;
> >>>>>          if (tag == MKTAG('h','d','l','r')) {
> >>>>>              avio_seek(pb, -8, SEEK_CUR);
> >>>>
> >>>> Maybe do something like "while (atom.size > 8 && !avio_feof(pb))"
> >>>> instead, which is similar to the loop in mov_read_default.
> >>>
> >>> Can do but why ?
> >>> the code in the patch returns an error if the atom is truncated
> >>> the change suggested does not return an error if the atom is truncated
> >>> on its own this doesnt sound better
> >>>
> >>> Thanks
> >>
> >> There's Marton's "avformat/utils: return pending IO error on EOF in
> >> av_read_frame()" patch to check in generic code if avio_feof() != 0 is
> >> an actual EOF or an IO error, so if you make this code here simply break
> >> the loop, same as it's done in mov_read_default(), then said generic
> >> code would be triggered and return the proper error code once the
> >> current packet is done processing.
> > 
> > are you against the original patch in this thread ?
> > from reading this its not clear to me if you dislike the original
> > patch or not ?
> > 
> > thanks
> 
> No, I'm not.

ok, will apply

thx

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190917/59fe58b7/attachment.sig>