On Sat, Jun 03, 2017 at 09:47:32PM -0300, James Almer wrote:
> On 6/3/2017 9:25 PM, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> > ---
> > libavcodec/utils.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index cde5849a41..feee7556ac 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -2278,6 +2278,9 @@ void avcodec_parameters_free(AVCodecParameters **ppar)
> >
> > int avcodec_parameters_copy(AVCodecParameters *dst, const
> > AVCodecParameters *src)
> > {
> > + if (src->extradata_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
> > + return AVERROR(EINVAL);
> > +
> > codec_parameters_reset(dst);
> > memcpy(dst, src, sizeof(*dst));
> >
> > @@ -2341,6 +2344,8 @@ int avcodec_parameters_from_context(AVCodecParameters
> > *par,
> > }
> >
> > if (codec->extradata) {
> > + if (codec->extradata_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
> > + return AVERROR(EINVAL);
> > par->extradata = av_mallocz(codec->extradata_size +
> > AV_INPUT_BUFFER_PADDING_SIZE);
> > if (!par->extradata)
> > return AVERROR(ENOMEM);
> > @@ -2397,6 +2402,8 @@ int avcodec_parameters_to_context(AVCodecContext
> > *codec,
> > }
> >
> > if (par->extradata) {
> > + if (par->extradata_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
> > + return AVERROR(EINVAL);
> > av_freep(&codec->extradata);
> > codec->extradata = av_mallocz(par->extradata_size +
> > AV_INPUT_BUFFER_PADDING_SIZE);
> > if (!codec->extradata)
> >
>
> ERANGE? Or ENOMEM, the only error these functions are currently
> returning. EINVAL for this situation with no way to log the reason of
> the error is not very intuitive. The function is meant to copy the
> fields from one struct to the other, not really validate said fields.
>
> I see EINVAL more fit as an error for example if src or dst are NULL,
> something that would actually be an invalid argument.
> We don't currently check that, for that matter. Maybe we should...
> do you prefer ERANGE or ENOMEM ? > Also, unless the user calls av_max_alloc to set a value higher than > INT_MAX, shouldn't av_mallocz refuse to alloc these? the size is a int, the addition would overflow and be a undefined operation. I dont have a testcase that triggers this though [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
