[FFmpeg-devel] Fix for PES packets with too much padding

[Michael Niedermayer]

On Thu, Aug 05, 2021 at 11:08:30PM +0200, Michael Niedermayer wrote:
> On Tue, Aug 03, 2021 at 10:07:34AM -0400, Sergio M. Ammirata, Ph.D. wrote:
> > PES packet with too much padding trigger unlimited error
> > messages "PES packet size mismatch" because the code that
> > corrects the length is wrong.
> > Here is a sample file: http://99.93.62.129/smpte2038.ts
> > PID 300 is the one triggering the errors.
> > I am attaching a patch that fixes the problem.
> > 
> 
> >  mpegts.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 8f8c73a5e78d7c740958054764defa3571979fc4  0001-Fix-setting-the-correct-size-for-PES-packets-with-to.patch
> > From 3a2760d42b38023c73f9a3ab18de01a44526dbc9 Mon Sep 17 00:00:00 2001
> > From: Sergio Ammirata <sergio at ammirata.net>
> > Date: Tue, 3 Aug 2021 13:43:44 +0000
> > Subject: [PATCH] Fix setting the correct size for PES packets with too much
> >  padding
> > 
> > ---
> >  libavformat/mpegts.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index 640c9afa5d..40439c26c0 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -1355,7 +1355,7 @@ skip:
> >                             buf_size > pes->total_size) {
> >                      // pes packet size is < ts size packet and pes data is padded with 0xff
> >                      // not sure if this is legal in ts but see issue #2392
> > -                    buf_size = pes->total_size;
> > +                    buf_size = PES_START_SIZE + pes->total_size - pes->pes_header_size;
> >                  }
> >                  memcpy(pes->buffer->data + pes->data_index, p, buf_size);
> >                  pes->data_index += buf_size;
> 
> This causes segfaults

==5552== Invalid write of size 2
==5552==    at 0x4C38753: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5552==    by 0x62DD54: mpegts_push_data (in ffmpeg_g)
==5552==    by 0x62B832: handle_packet (in ffmpeg_g)
==5552==    by 0x62BCD4: handle_packets (in ffmpeg_g)
==5552==    by 0x62BE11: mpegts_read_packet (in ffmpeg_g)
==5552==    by 0x6BADB9: ff_read_packet (in ffmpeg_g)
==5552==    by 0x6BBAFA: read_frame_internal (in ffmpeg_g)
==5552==    by 0x6C05EC: avformat_find_stream_info (in ffmpeg_g)
==5552==    by 0x2DB175: open_input_file (in ffmpeg_g)
==5552==    by 0x2DEA5B: ffmpeg_parse_options (in ffmpeg_g)
==5552==    by 0x2D3151: main (in ffmpeg_g)
==5552==  Address 0x2d25cb80 is 0 bytes after a block of size 128 alloc'd
==5552==    at 0x4C33E76: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5552==    by 0x4C33F91: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5552==    by 0x11166C2: av_malloc (in ffmpeg_g)
==5552==    by 0x1100F65: av_buffer_alloc (in ffmpeg_g)
==5552==    by 0x11017A4: av_buffer_pool_get (in ffmpeg_g)
==5552==    by 0x62E16F: mpegts_push_data (in ffmpeg_g)
==5552==    by 0x62B832: handle_packet (in ffmpeg_g)
==5552==    by 0x62BCD4: handle_packets (in ffmpeg_g)
==5552==    by 0x62BE11: mpegts_read_packet (in ffmpeg_g)
==5552==    by 0x6BADB9: ff_read_packet (in ffmpeg_g)
==5552==    by 0x6BBAFA: read_frame_internal (in ffmpeg_g)
==5552==    by 0x6C05EC: avformat_find_stream_info (in ffmpeg_g)
==5552==    by 0x2DB175: open_input_file (in ffmpeg_g)
==5552==    by 0x2DEA5B: ffmpeg_parse_options (in ffmpeg_g)
==5552==    by 0x2D3151: main (in ffmpeg_g)


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand
-------------- 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/20210805/938e68dd/attachment.sig>