Hi,
On Tue, Dec 13, 2016 at 8:21 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> On 14.12.2016 02:01, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Tue, Dec 13, 2016 at 7:57 PM, Andreas Cadhalpun <
> > andreas.cadhal...@googlemail.com> wrote:
> >
> >> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
> >> ---
> >> libavformat/4xm.c | 8 +++++++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/4xm.c b/libavformat/4xm.c
> >> index 8a50778..2758b69 100644
> >> --- a/libavformat/4xm.c
> >> +++ b/libavformat/4xm.c
> >> @@ -163,6 +163,12 @@ static int parse_strk(AVFormatContext *s,
> >> return AVERROR_INVALIDDATA;
> >> }
> >>
> >> + if (fourxm->tracks[track].sample_rate > INT64_MAX /
> >> fourxm->tracks[track].bits / fourxm->tracks[track].channels) {
> >> + av_log(s, AV_LOG_ERROR, "Overflow during bit rate calculation
> %d
> >> * %d * %d\n",
> >> + fourxm->tracks[track].sample_rate,
> >> fourxm->tracks[track].bits, fourxm->tracks[track].channels);
> >> + return AVERROR_INVALIDDATA;
> >> + }
> >
> >
> > What is the functional effect of the overflow?
>
> It is undefined behavior.
>
> > Does it crash? Or is there some other security issue?
>
> The most likely behavior is that bit_rate will contain a negative value,
> which might cause problems later on, but I'm not aware of specific
> security issues caused by this.
Not wanting to discourage you, but I wonder if there's really a point to
this...? I don't see how the user experience changes.
This isn't specifically intended at this patch, but rather at the sort of
rabbit hole this change might lead to, which would cause the code to be
uber-full of such checks, none of which really have any significance. But
maybe others disagree...
Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
