[FFmpeg-devel] Fix undefined behavior in ff_configure_buffers_for_index()

[Michael Niedermayer]

On Tue, Jan 28, 2020 at 04:52:16PM -0800, Dale Curtis wrote:
> When e2_pts == INT64_MIN and e1_pts >= 0 the calculation of
> e2_pts - e1_pts will overflow an int64_t. So instead check
> for overflow and default to |time_tolerance| if the value
> is too large for an int64_t.
> 
> Signed-off-by: Dale Curtis <dalecurtis at chromium.org>

>  utils.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 9c3670236229794d325158a25f26fd0cdf459310  ubfix.patch
> From 412751f4747faf34e3dba088dc55290783eb6bd5 Mon Sep 17 00:00:00 2001
> From: Dale Curtis <dalecurtis at chromium.org>
> Date: Tue, 28 Jan 2020 16:49:14 -0800
> Subject: [PATCH] Fix undefined behavior in ff_configure_buffers_for_index()
> 
> When e2_pts == INT64_MIN and e1_pts >= 0 the calculation of
> e2_pts - e1_pts will overflow an int64_t. So instead check
> for overflow and default to |time_tolerance| if the value
> is too large for an int64_t.
> 
> Signed-off-by: Dale Curtis <dalecurtis at chromium.org>
> ---
>  libavformat/utils.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index e22ca7cab8..d6197358c9 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -2135,7 +2135,13 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance)
>                  for (; i2 < st2->nb_index_entries; i2++) {
>                      AVIndexEntry *e2 = &st2->index_entries[i2];
>                      int64_t e2_pts = av_rescale_q(e2->timestamp, st2->time_base, AV_TIME_BASE_Q);
> -                    if (e2_pts - e1_pts < time_tolerance)
> +                    int64_t delta = e1_pts < 1 ? INT64_MAX + e1_pts >= e2_pts
> +                                                     ? e2_pts - e1_pts
> +                                                     : time_tolerance
> +                                               : INT64_MIN + e1_pts <= e2_pts
> +                                                     ? e2_pts - e1_pts
> +                                                     : time_tolerance;
> +                    if (delta < time_tolerance)
>                          continue;

simpler solution, and also behaves arithmetically more correct when the
overflow happens in the othert direction:

av_assert0(time_tolerance >= 0);

if (e2_pts < e1_pts || e2_pts - (uint64_t)e1_pts < time_tolerance)
    continue;

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20200129/ea2eefc4/attachment.sig>