[FFmpeg-devel] [PATCH 5/5] avcodec/hevc_mp4toannexb_bsf: Check that there is enough input left for nalu size

[Michael Niedermayer]

On Fri, Dec 13, 2019 at 06:05:21PM +0100, Andreas Rheinhardt wrote:
> On Fri, Dec 13, 2019 at 5:56 PM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > On Fri, Dec 13, 2019 at 12:24:04PM +0100, Andreas Rheinhardt wrote:
> > > On Fri, Dec 13, 2019 at 3:07 AM Michael Niedermayer
> > <michael at niedermayer.cc>
> > > wrote:
> > >
> > > > No testcase
> > > >
> > > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > > ---
> > > >  libavcodec/hevc_mp4toannexb_bsf.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c
> > > > b/libavcodec/hevc_mp4toannexb_bsf.c
> > > > index baa93628ed..e0d20a550c 100644
> > > > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > > > @@ -152,7 +152,9 @@ static int hevc_mp4toannexb_filter(AVBSFContext
> > *ctx,
> > > > AVPacket *out)
> > > >          extra_size    = add_extradata * ctx->par_out->extradata_size;
> > > >          got_irap     |= is_irap;
> > > >
> > > > -        if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size)
> > {
> > > > +        if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size
> > ||
> > > >
> > >
> > > Up until now I thought that FFmpeg has some implicit assumptions: int
> > > having 32bit being one of them (the log2 functions depend on this). And I
> >
> > yes, that was from POSIX
> >
> >
> > > also thought that size_t being able to hold all the values of an unsigned
> > > was one of these implicit assumptions, too. Am I wrong on this?
> >
> > I was asking myself the same, and i couldnt really find anything where we
> > stated that previously so i added a FFMIN.
> >
> > In this case we would have to add lots of checks before av_malloc and
> other allocation functions that use size_t parameters in order to ensure
> that no loss of information happens when an int (or unsigned) is converted
> to size_t. In other words: We should not support such systems.

If we would choose to support such platforms in the future then using our own
type thats the bigger of the 2 might make this relativly painless. But first
such a platform needs to be relevant for us and exist ...
And if we choose to assume things about these types, that should be in our
developer documentation.

But to return to the patch here, do you want me to remove the FFMIN ?

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/20191213/4b48d4b3/attachment.sig>