[FFmpeg-devel] Fix signed integer overflow in mov_write_single_packet Detected with clang and -fsanitize=signed-integer-overflow

Submitted by Vitaly Buka on Oct. 6, 2017, 11:20 p.m.

Details

Message ID 20171006232049.2405-1-vitalybuka@google.com
State New
Headers show

Commit Message

Vitaly Buka Oct. 6, 2017, 11:20 p.m.
Signed-off-by: Vitaly Buka <vitalybuka@google.com>
---
 libavformat/movenc.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Carl Eugen Hoyos Oct. 7, 2017, 12:05 a.m.
2017-10-07 1:20 GMT+02:00 Vitaly Buka <vitalybuka-at-google.com@ffmpeg.org>:
> Signed-off-by: Vitaly Buka <vitalybuka@google.com>
> ---
>  libavformat/movenc.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 2838286141..e70500ae2c 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -5354,6 +5354,10 @@ static int mov_write_single_packet(AVFormatContext *s, AVPacket *pkt)
>                  // duration, but only helps for this particular track, not
>                  // for the other ones that are flushed at the same time.
>                  trk->track_duration = pkt->dts - trk->start_dts;
> +                if (trk->start_dts != AV_NOPTS_VALUE)
> +                    trk->track_duration = pkt->dts - trk->start_dts;
> +                else
> +                    trk->track_duration = 0;

I suspect you wanted to remove the line immediately
before the new lines, no?
Please consider adding braces around the else.

Carl Eugen
Vitaly Buka Oct. 29, 2017, 4:36 a.m.
ping

On Fri, Oct 6, 2017 at 4:20 PM, Vitaly Buka <vitalybuka@google.com> wrote:

> Signed-off-by: Vitaly Buka <vitalybuka@google.com>
> ---
>  libavformat/movenc.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 2838286141..e70500ae2c 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -5354,6 +5354,10 @@ static int mov_write_single_packet(AVFormatContext
> *s, AVPacket *pkt)
>                  // duration, but only helps for this particular track, not
>                  // for the other ones that are flushed at the same time.
>                  trk->track_duration = pkt->dts - trk->start_dts;
> +                if (trk->start_dts != AV_NOPTS_VALUE)
> +                    trk->track_duration = pkt->dts - trk->start_dts;
> +                else
> +                    trk->track_duration = 0;
>                  if (pkt->pts != AV_NOPTS_VALUE)
>                      trk->end_pts = pkt->pts;
>                  else
> --
> 2.14.2.920.gcf0c67979c-goog
>
>
Moritz Barsnick Oct. 29, 2017, 1:53 p.m.
On Sat, Oct 28, 2017 at 21:36:10 -0700, Vitaly Buka wrote:
> ping

You didn't respond to Carl Eugen's review.

Furthermore, the second line of your commit message ("Detected...")
should be separated from the first with an empty line.

Moritz

Patch hide | download patch | download mbox

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 2838286141..e70500ae2c 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5354,6 +5354,10 @@  static int mov_write_single_packet(AVFormatContext *s, AVPacket *pkt)
                 // duration, but only helps for this particular track, not
                 // for the other ones that are flushed at the same time.
                 trk->track_duration = pkt->dts - trk->start_dts;
+                if (trk->start_dts != AV_NOPTS_VALUE)
+                    trk->track_duration = pkt->dts - trk->start_dts;
+                else
+                    trk->track_duration = 0;
                 if (pkt->pts != AV_NOPTS_VALUE)
                     trk->end_pts = pkt->pts;
                 else