From 548bc3a46d54711d974aae9ce1bce291376c0436 Mon Sep 17 00:00:00 2001 From: Jacob Boerema Date: Thu, 1 May 2025 12:42:17 -0400 Subject: [PATCH] plug-ins: CWE-190: Integer Overflow or Wraparound in Despeckle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As reported by Seungho Kim our despeckle filter doesn't check for integer overflow when allocating buffers, nor do we check for failed allocations. A potential integer overflow vulnerability exists in the GIMP "Despeckle" plug-in. The issue occurs due to unchecked multiplication of image dimensions (width, height) and bytes-per-pixel (img_bpp), which can result in allocating insufficient memory and subsequently performing out-of-bounds writes. This could lead to heap corruption and potential denial-of-service (DoS) or arbitrary code execution in certain scenarios. Vulnerability Details •width and height are of type guint (signed 32-bit int). •Multiplying width * height * img_bpp can result in a value exceeding the bounds of gsize. •g_new() does not perform overflow protection; if the size wraps around, less memory than needed will be allocated. •Subsequent pixel processing loops write beyond the allocated memory region (src, dst). Proof of Concept (PoC) Open a specially crafted image with very large dimensions (e.g., 70,000 x 70,000 pixels) and apply the Despeckle filter. GIMP may crash due to heap corruption, or undefined behavior may occur. We applied the suggested changes and in addition adjusted the despeckle function to be able to set error messages, and check for NULL allocations. --- plug-ins/common/despeckle.c | 62 +++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/plug-ins/common/despeckle.c b/plug-ins/common/despeckle.c index 3250925b94..ffa24f06c6 100644 --- a/plug-ins/common/despeckle.c +++ b/plug-ins/common/despeckle.c @@ -98,8 +98,9 @@ static GimpValueArray * despeckle_run (GimpProcedure *proced GimpProcedureConfig *config, gpointer run_data); -static void despeckle (GimpDrawable *drawable, - GObject *config); +static gboolean despeckle (GimpDrawable *drawable, + GObject *config, + GError **error); static void despeckle_median (GObject *config, guchar *src, guchar *dst, @@ -224,13 +225,12 @@ despeckle_run (GimpProcedure *procedure, gpointer run_data) { GimpDrawable *drawable; + GError *error = NULL; gegl_init (NULL, NULL); if (gimp_core_object_array_get_length ((GObject **) drawables) != 1) { - GError *error = NULL; - g_set_error (&error, GIMP_PLUG_IN_ERROR, 0, _("Procedure '%s' only works with one drawable."), PLUG_IN_PROC); @@ -250,7 +250,10 @@ despeckle_run (GimpProcedure *procedure, if (run_mode == GIMP_RUN_INTERACTIVE && ! despeckle_dialog (procedure, G_OBJECT (config), drawable)) return gimp_procedure_new_return_values (procedure, GIMP_PDB_CANCEL, NULL); - despeckle (drawable, G_OBJECT (config)); + if (! despeckle (drawable, G_OBJECT (config), &error)) + return gimp_procedure_new_return_values (procedure, + GIMP_PDB_EXECUTION_ERROR, + error); return gimp_procedure_new_return_values (procedure, GIMP_PDB_SUCCESS, NULL); } @@ -323,9 +326,10 @@ get_u8_format (GimpDrawable *drawable) } } -static void +static gboolean despeckle (GimpDrawable *drawable, - GObject *config) + GObject *config, + GError **error) { GeglBuffer *src_buffer; GeglBuffer *dest_buffer; @@ -335,10 +339,11 @@ despeckle (GimpDrawable *drawable, gint img_bpp; gint x, y; gint width, height; + gsize bufsize = 0; if (! gimp_drawable_mask_intersect (drawable, &x, &y, &width, &height)) - return; + return TRUE; format = get_u8_format (drawable); img_bpp = babl_format_get_bytes_per_pixel (format); @@ -346,8 +351,26 @@ despeckle (GimpDrawable *drawable, src_buffer = gimp_drawable_get_buffer (drawable); dest_buffer = gimp_drawable_get_shadow_buffer (drawable); - src = g_new (guchar, width * height * img_bpp); - dst = g_new (guchar, width * height * img_bpp); + if (! g_size_checked_mul (&bufsize, width, height) || + ! g_size_checked_mul (&bufsize, bufsize, img_bpp)) + { + g_set_error (error, GIMP_PLUG_IN_ERROR, 0, + _("Image dimensions too large: width %d x height %d"), + width, height); + return FALSE; + } + + src = g_try_malloc (bufsize); + dst = g_try_malloc (bufsize); + + if (src == NULL || dst == NULL) + { + g_set_error (error, GIMP_PLUG_IN_ERROR, 0, + _("There was not enough memory to complete the operation.")); + g_free (src); + + return FALSE; + } gegl_buffer_get (src_buffer, GEGL_RECTANGLE (x, y, width, height), 1.0, format, src, @@ -368,6 +391,8 @@ despeckle (GimpDrawable *drawable, g_free (dst); g_free (src); + + return TRUE; } static gboolean @@ -446,8 +471,9 @@ static void preview_update (GtkWidget *widget, GObject *config) { - GimpPreview *preview = GIMP_PREVIEW (widget); + GimpPreview *preview = GIMP_PREVIEW (widget); GimpDrawable *drawable = g_object_get_data (config, "drawable"); + gsize bufsize = 0; GeglBuffer *src_buffer; const Babl *format; guchar *dst; @@ -464,8 +490,18 @@ preview_update (GtkWidget *widget, src_buffer = gimp_drawable_get_buffer (drawable); - dst = g_new (guchar, width * height * img_bpp); - src = g_new (guchar, width * height * img_bpp); + if (! g_size_checked_mul (&bufsize, width, height) || + ! g_size_checked_mul (&bufsize, bufsize, img_bpp)) + return; + + src = g_try_malloc (bufsize); + dst = g_try_malloc (bufsize); + + if (src == NULL || dst == NULL) + { + g_free (src); + return; + } gegl_buffer_get (src_buffer, GEGL_RECTANGLE (x1, y1, width, height), 1.0, format, src,