[FFmpeg-devel] [PATCH] avformat/mpegts: Shuffle avio_seek

[Michael Niedermayer]

On Thu, May 07, 2020 at 02:09:43PM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-05-07 12:38:26)
> > This avoids accessing an old, no longer valid buffer.
> > Fixes: out of array access
> > Fixes: crash_audio-2020
> > 
> > Found-by: le wu <shoulewoba at gmail.com>
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavformat/mpegts.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index 0833d62ea5..a065c61c40 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -2881,15 +2881,16 @@ static int mpegts_resync(AVFormatContext *s, int seekback, const uint8_t *curren
> >      AVIOContext *pb = s->pb;
> >      int c, i;
> >      uint64_t pos = avio_tell(pb);
> > -
> > -    avio_seek(pb, -FFMIN(seekback, pos), SEEK_CUR);
> > +    int64_t back = FFMIN(seekback, pos);
> >  
> >      //Special case for files like 01c56b0dc1.ts
> >      if (current_packet[0] == 0x80 && current_packet[12] == 0x47) {
> > -        avio_seek(pb, 12, SEEK_CUR);
> > +        avio_seek(pb, 12 - back, SEEK_CUR);
> >          return 0;
> >      }
> >  
> > +    avio_seek(pb, -back, SEEK_CUR);
> > +
> 
> This seems pretty non-obvious - why would ordering seeks in a specific
> way result in invalid memorry access?

because current_packet in one case points to the avio internal buffer
and doing any "state changing" avio could change that buffer.
so all accesses to the buffer must happen before any avio seeks

this issue in fact was not reproducable in master but some release branches
it just seems master is affected too. For reproduction a custom http server
may be required, i did had some initial difficulty with reproduction even on
the affected release branch ...

thx

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

Elect your leaders based on what they did after the last election, not
based on what they say before an election.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200507/b151b4f9/attachment.sig>