[FFmpeg-devel] [PATCH] Avoid undefined behavior by limiting PTS to 62 bits in ogg decoder

[Fredrik Hubinette]

I'm uncertain what the right thing to do here is.
Oggdec generates PTS values which are a bit crazy, and the generic code
that deals which those values encounters undefined behavior.
It seems like there should be some input validation happening somewhere,
but it's not clear to me if that belongs in oggdec or somewhere else.


On Mon, Jul 30, 2018 at 4:23 PM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Mon, Jul 30, 2018 at 01:31:59PM -0700, Fredrik Hubinette wrote:
> > Not sure how to update the commit message.
>
> git commit --amend
>
>
> > The undefined behavior occurs in av_add_stable, which is called from
> > compute_packet_fields.
> > In that code PTS can be equal to -(1<<63), which then causes a int64_t to
> > overflow.
>
> This does not sound specific to oggdec. Thus a change in oggdec would
> likely
> not fix it outside ogg.
> Or am i missing something ?
>
>
> thx
>
> >
> > On Wed, Jul 18, 2018 at 4:04 AM Michael Niedermayer
> <michael at niedermayer.cc>
> > wrote:
> >
> > > On Mon, Jul 16, 2018 at 04:32:14PM -0700, Fredrik Hubinette wrote:
> > > > With some (garbled) OGG data, PTS can overflow causing undefined
> > > behavior.
> > > > This patch avoids that by zeroing out PTS values greater than 2^62.
> > >
> > > >  oggdec.h |    3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > ff003b78842c7724ccc1a42f9584b1f8aa0b0b3d
> > > 0001-Avoid-undefined-behavior-by-limiting-PTS-to-62-bits-.patch
> > > > From 26a8582bc04f5bddc037ffcce99025e2f977abe0 Mon Sep 17 00:00:00
> 2001
> > > > From: Fredrik Hubinette <hubbe at google.com>
> > > > Date: Mon, 16 Jul 2018 14:54:43 -0700
> > > > Subject: [PATCH] Avoid undefined behavior by limiting PTS to 62 bits
> in
> > > ogg
> > > >  decoder
> > > >
> > > > ---
> > > >  libavformat/oggdec.h | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > I think someone reading this commit message would like to know more
> > > about where that undefined behaviour occurs and how this is guranteeing
> > > to fix it
> > >
> > >
> > > [...]
> > >
> > > --
> > > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > >
> > > "Nothing to hide" only works if the folks in power share the values of
> > > you and everyone you know entirely and always will -- Tom Scott
> > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Its not that you shouldnt use gotos but rather that you should write
> readable code and code with gotos often but not always is less readable
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>