[FFmpeg-devel] Fix float-cast-overflow undefined behavior in matroskadec

Submitted by Nikolas Bowe on Jan. 18, 2018, 11:21 p.m.

Details

Message ID 20180118232156.108823-1-nbowe@google.com
State New
Headers show

Commit Message

Nikolas Bowe Jan. 18, 2018, 11:21 p.m.
---
 libavformat/matroskadec.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Carl Eugen Hoyos Jan. 19, 2018, 12:02 a.m.
2018-01-19 0:21 GMT+01:00 Nikolas Bowe <nbowe-at-google.com@ffmpeg.org>:

> +                  av_log(matroska->ctx, AV_LOG_WARNING,
> +                         "Invalid frame rate %e. Cannot calculate default duration.\n",
> +                         track->video.frame_rate);

We currently have one ocurence of "%e" in the whole codebase
and it is usually not compiled: Can you use %f here or would that
be a disadvantage?

Carl Eugen
Nikolas Bowe Jan. 19, 2018, 1:06 a.m.
In the file the fuzzer produced the frame rate is incredibly small
(7.29112e-304).
I thought %e (or %g if you prefer) produced the most appropriate and
readable log message.
Also happy to just remove the frame rate from the log if you prefer?

On Thu, Jan 18, 2018 at 4:02 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
wrote:

> 2018-01-19 0:21 GMT+01:00 Nikolas Bowe <nbowe-at-google.com@ffmpeg.org>:
>
> > +                  av_log(matroska->ctx, AV_LOG_WARNING,
> > +                         "Invalid frame rate %e. Cannot calculate
> default duration.\n",
> > +                         track->video.frame_rate);
>
> We currently have one ocurence of "%e" in the whole codebase
> and it is usually not compiled: Can you use %f here or would that
> be a disadvantage?
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Carl Eugen Hoyos Jan. 19, 2018, 11:29 a.m.
2018-01-19 2:06 GMT+01:00 Niki Bowe <nbowe-at-google.com@ffmpeg.org>:
> In the file the fuzzer produced the frame rate is incredibly small
> (7.29112e-304).

And the msvc documentation seems to indicate it is supported.

No more comments from me, please avoid top-posting here.

Carl Eugen
Michael Niedermayer Jan. 19, 2018, 5:05 p.m.
On Thu, Jan 18, 2018 at 03:21:56PM -0800, Nikolas Bowe wrote:
> ---
>  libavformat/matroskadec.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)

will apply with the indention and commit message fixed

thanks

[...]

Patch hide | download patch | download mbox

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 496499b553..cd9e1f56c2 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2096,8 +2096,16 @@  static int matroska_parse_tracks(AVFormatContext *s)
         }
 
         if (track->type == MATROSKA_TRACK_TYPE_VIDEO) {
-            if (!track->default_duration && track->video.frame_rate > 0)
-                track->default_duration = 1000000000 / track->video.frame_rate;
+            if (!track->default_duration && track->video.frame_rate > 0) {
+                double default_duration = 1000000000 / track->video.frame_rate;
+                if (default_duration > UINT64_MAX || default_duration < 0) {
+                  av_log(matroska->ctx, AV_LOG_WARNING,
+                         "Invalid frame rate %e. Cannot calculate default duration.\n",
+                         track->video.frame_rate);
+                } else {
+                  track->default_duration = default_duration;
+                }
+            }
             if (track->video.display_width == -1)
                 track->video.display_width = track->video.pixel_width;
             if (track->video.display_height == -1)