From: Jan Schmidt <jan@centricular.com>
Date: Fri, 5 Jun 2020 15:22:21 +0000 (+1000)
Subject: video-converter: Add checks for configuration sanity.
X-Git-Tag: 1.19.3~511^2~567
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=205bb066edb6b87fdb3546eb9cdfb04640c93310;p=platform%2Fupstream%2Fgstreamer.git

video-converter: Add checks for configuration sanity.

If the cropping or scaling input or output rects put us completely
outside the input/output frame respectively, we can't draw anything
except black safely. Check for those conditions and don't set up a
configuration that attempts to access out of bounds memory outside
the input/output framebuffers.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/696>
---

diff --git a/gst-libs/gst/video/video-converter.c b/gst-libs/gst/video/video-converter.c
index 3f5fa44..c89932a 100644
--- a/gst-libs/gst/video/video-converter.c
+++ b/gst-libs/gst/video/video-converter.c
@@ -2335,8 +2335,17 @@ gst_video_converter_new (GstVideoInfo * in_info, GstVideoInfo * out_info,
 
   convert->in_width =
       MIN (convert->in_width, convert->in_maxwidth - convert->in_x);
+  if (convert->in_width + convert->in_x < 0 ||
+      convert->in_width + convert->in_x > convert->in_maxwidth) {
+    convert->in_width = 0;
+  }
+
   convert->in_height =
       MIN (convert->in_height, convert->in_maxheight - convert->in_y);
+  if (convert->in_height + convert->in_y < 0 ||
+      convert->in_height + convert->in_y > convert->in_maxheight) {
+    convert->in_height = 0;
+  }
 
   convert->out_x = get_opt_int (convert, GST_VIDEO_CONVERTER_OPT_DEST_X, 0);
   convert->out_y = get_opt_int (convert, GST_VIDEO_CONVERTER_OPT_DEST_Y, 0);
@@ -2350,10 +2359,25 @@ gst_video_converter_new (GstVideoInfo * in_info, GstVideoInfo * out_info,
       get_opt_int (convert, GST_VIDEO_CONVERTER_OPT_DEST_HEIGHT,
       convert->out_maxheight - convert->out_y);
 
-  convert->out_width =
-      MIN (convert->out_width, convert->out_maxwidth - convert->out_x);
-  convert->out_height =
-      MIN (convert->out_height, convert->out_maxheight - convert->out_y);
+  if (convert->out_width > convert->out_maxwidth - convert->out_x)
+    convert->out_width = convert->out_maxwidth - convert->out_x;
+  convert->out_width = CLAMP (convert->out_width, 0, convert->out_maxwidth);
+
+  /* Check if completely outside the framebuffer */
+  if (convert->out_width + convert->out_x < 0 ||
+      convert->out_width + convert->out_x > convert->out_maxwidth) {
+    convert->out_width = 0;
+  }
+
+  /* Same for height */
+  if (convert->out_height > convert->out_maxheight - convert->out_y)
+    convert->out_height = convert->out_maxheight - convert->out_y;
+  convert->out_height = CLAMP (convert->out_height, 0, convert->out_maxheight);
+
+  if (convert->out_height + convert->out_y < 0 ||
+      convert->out_height + convert->out_y > convert->out_maxheight) {
+    convert->out_height = 0;
+  }
 
   convert->fill_border = GET_OPT_FILL_BORDER (convert);
   convert->border_argb = get_opt_uint (convert,
@@ -2396,6 +2420,9 @@ gst_video_converter_new (GstVideoInfo * in_info, GstVideoInfo * out_info,
   /* Magic number of 200 lines */
   if (MAX (convert->out_height, convert->in_height) / n_threads < 200)
     n_threads = (MAX (convert->out_height, convert->in_height) + 199) / 200;
+  if (n_threads < 1)
+    n_threads = 1;
+
   convert->conversion_runner = gst_parallelized_task_runner_new (n_threads);
 
   if (video_converter_lookup_fastpath (convert))
@@ -2429,33 +2456,36 @@ gst_video_converter_new (GstVideoInfo * in_info, GstVideoInfo * out_info,
   convert->dither_lines = g_new0 (GstLineCache *, n_threads);
   convert->dither = g_new0 (GstVideoDither *, n_threads);
 
-  for (i = 0; i < n_threads; i++) {
-    convert->current_format = GST_VIDEO_INFO_FORMAT (in_info);
-    convert->current_width = convert->in_width;
-    convert->current_height = convert->in_height;
-
-    /* unpack */
-    prev = chain_unpack_line (convert, i);
-    /* upsample chroma */
-    prev = chain_upsample (convert, prev, i);
-    /* convert to gamma decoded RGB */
-    prev = chain_convert_to_RGB (convert, prev, i);
-    /* do all downscaling */
-    prev = chain_scale (convert, prev, FALSE, i);
-    /* do conversion between color spaces */
-    prev = chain_convert (convert, prev, i);
-    /* do alpha channels */
-    prev = chain_alpha (convert, prev, i);
-    /* do all remaining (up)scaling */
-    prev = chain_scale (convert, prev, TRUE, i);
-    /* convert to gamma encoded Y'Cb'Cr' */
-    prev = chain_convert_to_YUV (convert, prev, i);
-    /* downsample chroma */
-    prev = chain_downsample (convert, prev, i);
-    /* dither */
-    prev = chain_dither (convert, prev, i);
-    /* pack into final format */
-    convert->pack_lines[i] = chain_pack (convert, prev, i);
+  if (convert->in_width > 0 && convert->out_width > 0 && convert->in_height > 0
+      && convert->out_height > 0) {
+    for (i = 0; i < n_threads; i++) {
+      convert->current_format = GST_VIDEO_INFO_FORMAT (in_info);
+      convert->current_width = convert->in_width;
+      convert->current_height = convert->in_height;
+
+      /* unpack */
+      prev = chain_unpack_line (convert, i);
+      /* upsample chroma */
+      prev = chain_upsample (convert, prev, i);
+      /* convert to gamma decoded RGB */
+      prev = chain_convert_to_RGB (convert, prev, i);
+      /* do all downscaling */
+      prev = chain_scale (convert, prev, FALSE, i);
+      /* do conversion between color spaces */
+      prev = chain_convert (convert, prev, i);
+      /* do alpha channels */
+      prev = chain_alpha (convert, prev, i);
+      /* do all remaining (up)scaling */
+      prev = chain_scale (convert, prev, TRUE, i);
+      /* convert to gamma encoded Y'Cb'Cr' */
+      prev = chain_convert_to_YUV (convert, prev, i);
+      /* downsample chroma */
+      prev = chain_downsample (convert, prev, i);
+      /* dither */
+      prev = chain_dither (convert, prev, i);
+      /* pack into final format */
+      convert->pack_lines[i] = chain_pack (convert, prev, i);
+    }
   }
 
   setup_borderline (convert);
@@ -2695,6 +2725,10 @@ gst_video_converter_frame (GstVideoConverter * convert,
     return;
   }
 
+  if (G_UNLIKELY (convert->in_width == 0 || convert->in_height == 0 ||
+          convert->out_width == 0 || convert->out_height == 0))
+    return;
+
   convert->convert (convert, src, dest);
 }
 
diff --git a/tests/check/libs/video.c b/tests/check/libs/video.c
index 97b316f..8f78395 100644
--- a/tests/check/libs/video.c
+++ b/tests/check/libs/video.c
@@ -2691,7 +2691,23 @@ GST_START_TEST (test_video_convert)
           320));
   gst_video_frame_map (&inframe, &ininfo, inbuffer, GST_MAP_READ);
   ASSERT_CRITICAL (gst_video_converter_frame (convert, &inframe, &outframe));
+  gst_video_converter_free (convert);
 
+  /* Make sure we can crop the entire frame away without dying */
+  convert = gst_video_converter_new (&ininfo, &outinfo,
+      gst_structure_new ("options",
+          GST_VIDEO_CONVERTER_OPT_RESAMPLER_METHOD,
+          GST_TYPE_VIDEO_RESAMPLER_METHOD, 3,
+          GST_VIDEO_CONVERTER_OPT_SRC_X, G_TYPE_INT, -500,
+          GST_VIDEO_CONVERTER_OPT_SRC_Y, G_TYPE_INT, -500,
+          GST_VIDEO_CONVERTER_OPT_SRC_WIDTH, G_TYPE_INT, 300,
+          GST_VIDEO_CONVERTER_OPT_SRC_HEIGHT, G_TYPE_INT, 220,
+          GST_VIDEO_CONVERTER_OPT_DEST_X, G_TYPE_INT, 800,
+          GST_VIDEO_CONVERTER_OPT_DEST_Y, G_TYPE_INT, 600,
+          GST_VIDEO_CONVERTER_OPT_DEST_WIDTH, G_TYPE_INT, 310,
+          GST_VIDEO_CONVERTER_OPT_DEST_HEIGHT, G_TYPE_INT, 230, NULL));
+
+  gst_video_converter_frame (convert, &inframe, &outframe);
   gst_video_converter_free (convert);
 
   gst_video_frame_unmap (&outframe);