diff mbox

[FFmpeg-devel,1/7] avcodec/ffv1dec: add missing error messages when a frame is invalid

Message ID 5d1d2226-1a19-5268-dcdd-441d551b1809@mediaarea.net
State New
Headers show

Commit Message

Jerome Martinez March 8, 2018, 1:48 a.m. UTC 
On 08/03/2018 01:17, Michael Niedermayer wrote:
>
> In the cases where the error is about a scalar value, that value should
> be printed in the error message (unless it was alread printed elsewhere)

Patch modified, showing the scalar value.
From 13db9fc4da1d0e531e516bd87d084233e231f0e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net>
Date: Thu, 8 Mar 2018 02:40:21 +0100
Subject: [PATCH] avcodec/ffv1dec: add missing error messages when a frame is
 invalid

---
 libavcodec/ffv1dec.c | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

Comments

Michael Niedermayer March 9, 2018, 11:30 p.m. UTC | #1 
On Thu, Mar 08, 2018 at 02:48:19AM +0100, Jerome Martinez wrote:
> On 08/03/2018 01:17, Michael Niedermayer wrote:
> >
> >In the cases where the error is about a scalar value, that value should
> >be printed in the error message (unless it was alread printed elsewhere)
> 
> Patch modified, showing the scalar value.

>  ffv1dec.c |   39 +++++++++++++++++++++++++++++++--------
>  1 file changed, 31 insertions(+), 8 deletions(-)
> b91dc1d7d73ea648cb343b5c3ed8159dc9bcc015  0001-avcodec-ffv1dec-add-missing-error-messages-when-a-fr.patch
> From 13db9fc4da1d0e531e516bd87d084233e231f0e7 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net>
> Date: Thu, 8 Mar 2018 02:40:21 +0100
> Subject: [PATCH] avcodec/ffv1dec: add missing error messages when a frame is
>  invalid
> 
> ---
>  libavcodec/ffv1dec.c | 39 +++++++++++++++++++++++++++++++--------
>  1 file changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
> index 3d2ee2569f..06509dae60 100644
> --- a/libavcodec/ffv1dec.c
> +++ b/libavcodec/ffv1dec.c
> @@ -182,11 +182,22 @@ static int decode_slice_header(FFV1Context *f, FFV1Context *fs)
>      fs->slice_y /= f->num_v_slices;
>      fs->slice_width  = fs->slice_width /f->num_h_slices - fs->slice_x;
>      fs->slice_height = fs->slice_height/f->num_v_slices - fs->slice_y;
> -    if ((unsigned)fs->slice_width > f->width || (unsigned)fs->slice_height > f->height)
> +    if ((unsigned)fs->slice_width > f->width) {
> +        av_log(f->avctx, AV_LOG_ERROR, "slice_width %d out of range\n", (unsigned)fs->slice_width);
>          return -1;
> -    if (    (unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width
> -         || (unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height)
> +    }
> +    if ((unsigned)fs->slice_height > f->height) {
> +        av_log(f->avctx, AV_LOG_ERROR, "slice_height %d out of range\n", (unsigned)fs->slice_height);
> +        return -1;
> +    }
> +    if ((unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width) {
> +        av_log(f->avctx, AV_LOG_ERROR, "slice_x+slice_width %lld out of range\n", (unsigned)fs->slice_x + (uint64_t)fs->slice_width, (unsigned)fs->slice_y);

%lld is incorrect for uint64_t
PRIu64 or PRId64 or something liek that is matching it

thx

[...]
diff mbox

Patch

diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 3d2ee2569f..06509dae60 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -182,11 +182,22 @@  static int decode_slice_header(FFV1Context *f, FFV1Context *fs)
     fs->slice_y /= f->num_v_slices;
     fs->slice_width  = fs->slice_width /f->num_h_slices - fs->slice_x;
     fs->slice_height = fs->slice_height/f->num_v_slices - fs->slice_y;
-    if ((unsigned)fs->slice_width > f->width || (unsigned)fs->slice_height > f->height)
+    if ((unsigned)fs->slice_width > f->width) {
+        av_log(f->avctx, AV_LOG_ERROR, "slice_width %d out of range\n", (unsigned)fs->slice_width);
         return -1;
-    if (    (unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width
-         || (unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height)
+    }
+    if ((unsigned)fs->slice_height > f->height) {
+        av_log(f->avctx, AV_LOG_ERROR, "slice_height %d out of range\n", (unsigned)fs->slice_height);
+        return -1;
+    }
+    if ((unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width) {
+        av_log(f->avctx, AV_LOG_ERROR, "slice_x+slice_width %lld out of range\n", (unsigned)fs->slice_x + (uint64_t)fs->slice_width, (unsigned)fs->slice_y);
         return -1;
+    }
+    if ((unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height) {
+        av_log(f->avctx, AV_LOG_ERROR, "slice_y+slice_height %lld out of range\n", (unsigned)fs->slice_y + (uint64_t)fs->slice_height);
+        return -1;
+    }
 
     for (i = 0; i < f->plane_count; i++) {
         PlaneContext * const p = &fs->plane[i];
@@ -432,8 +443,10 @@  static int read_extra_header(FFV1Context *f)
     if (f->version > 2) {
         c->bytestream_end -= 4;
         f->micro_version = get_symbol(c, state, 0);
-        if (f->micro_version < 0)
+        if (f->micro_version < 0) {
+            av_log(f->avctx, AV_LOG_ERROR, "Invalid micro_version %i in global header\n", f->micro_version);
             return AVERROR_INVALIDDATA;
+        }
     }
     f->ac = get_symbol(c, state, 0);
 
@@ -758,12 +771,22 @@  static int read_header(FFV1Context *f)
             fs->slice_y     /= f->num_v_slices;
             fs->slice_width  = fs->slice_width  / f->num_h_slices - fs->slice_x;
             fs->slice_height = fs->slice_height / f->num_v_slices - fs->slice_y;
-            if ((unsigned)fs->slice_width  > f->width ||
-                (unsigned)fs->slice_height > f->height)
+            if ((unsigned)fs->slice_width  > f->width) {
+                av_log(f->avctx, AV_LOG_ERROR, "slice_width %d out of range\n", (unsigned)fs->slice_width);
                 return AVERROR_INVALIDDATA;
-            if (   (unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width
-                || (unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height)
+            }
+            if ((unsigned)fs->slice_height > f->height) {
+                av_log(f->avctx, AV_LOG_ERROR, "slice_height %d out of range\n", (unsigned)fs->slice_height);
+                return AVERROR_INVALIDDATA;
+            }
+            if ((unsigned)fs->slice_x + (uint64_t)fs->slice_width  > f->width) {
+                av_log(f->avctx, AV_LOG_ERROR, "slice_x+slice_width %lld out of range\n", (unsigned)fs->slice_x + (uint64_t)fs->slice_width, (unsigned)fs->slice_y);
                 return AVERROR_INVALIDDATA;
+            }
+            if ((unsigned)fs->slice_y + (uint64_t)fs->slice_height > f->height) {
+                av_log(f->avctx, AV_LOG_ERROR, "slice_y+slice_height %lld out of range\n", (unsigned)fs->slice_y + (uint64_t)fs->slice_height);
+                return AVERROR_INVALIDDATA;
+            }
         }
 
         for (i = 0; i < f->plane_count; i++) {