From b7219d51b150fa10018ccc810824940ea56284f1 Mon Sep 17 00:00:00 2001
From: Jacob Boerema <jgboerema@gmail.com>
Date: Tue, 8 Nov 2022 14:10:05 -0500
Subject: [PATCH] plug-ins: improve security in flame plug-in
- Use g_malloc* functions instead of malloc, so we don't continue on
failed allocations unless we test for NULL.
- Make sure we don't iterate past the known number of control points (ncps).
- Safely allocate, initialize and free points. Since points seems to be
used uninitialized, we use g_malloc0 to set everything to 0.
(cherry picked from commit 981979bb39a9453f33d8c33f12ef19ff15be25ba)
---
plug-ins/flame/libifs.c | 17 +++++++++++++----
plug-ins/flame/rect.c | 13 +++++++------
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/plug-ins/flame/libifs.c b/plug-ins/flame/libifs.c
index 16259d46a5..461ac5c96d 100644
--- a/plug-ins/flame/libifs.c
+++ b/plug-ins/flame/libifs.c
@@ -692,6 +692,8 @@ interpolate (control_point cps[],
int i, j, i1, i2;
double c0, c1, t;
+ g_return_if_fail (ncps > 0);
+
if (ncps == 1)
{
*result = cps[0];
@@ -710,12 +712,14 @@ interpolate (control_point cps[],
else
{
i1 = 0;
- while (cps[i1].time < time)
+ while (i1 < ncps && cps[i1].time < time)
i1++;
i1--;
i2 = i1 + 1;
- if (time - cps[i1].time > -1e-7 &&
- time - cps[i1].time < 1e-7)
+
+ if (i2 == ncps ||
+ (time - cps[i1].time > -1e-7 &&
+ time - cps[i1].time < 1e-7))
{
*result = cps[i1];
return;
@@ -1373,7 +1380,8 @@ estimate_bounding_box (control_point *cp,
int low_target = batch * eps;
int high_target = batch - low_target;
point min, max, delta;
- point *points = malloc (sizeof (point) * batch);
+ point *points = g_malloc0 (sizeof (point) * batch);
+
iterate (cp, batch, 20, points);
min[0] = min[1] = 1e10;
@@ -1420,6 +1428,7 @@ estimate_bounding_box (control_point *cp,
delta[0] = delta[0] / 2.0;
delta[1] = delta[1] / 2.0;
}
+ g_free (points);
}
/* this has serious flaws in it */
diff --git a/plug-ins/flame/rect.c b/plug-ins/flame/rect.c
index 4951bfdfc8..0ff0f5f89c 100644
--- a/plug-ins/flame/rect.c
+++ b/plug-ins/flame/rect.c
@@ -20,6 +20,7 @@
#include <string.h>
+#include "libgimp/gimp.h"
/* for batch
* interpolate
@@ -122,7 +123,7 @@ render_rectangle (frame_spec *spec,
if ((filter_width ^ oversample) & 1)
filter_width++;
- filter = malloc (sizeof (double) * filter_width * filter_width);
+ filter = g_malloc (sizeof (double) * filter_width * filter_width);
/* fill in the coefs */
for (i = 0; i < filter_width; i++)
for (j = 0; j < filter_width; j++)
@@ -135,8 +136,8 @@ render_rectangle (frame_spec *spec,
}
normalize_vector(filter, filter_width * filter_width);
}
- temporal_filter = malloc (sizeof (double) * nbatches);
- temporal_deltas = malloc (sizeof (double) * nbatches);
+ temporal_filter = g_malloc (sizeof (double) * nbatches);
+ temporal_deltas = g_malloc (sizeof (double) * nbatches);
if (nbatches > 1)
{
double t;
@@ -173,11 +174,11 @@ render_rectangle (frame_spec *spec,
{
if (last_block != NULL)
free (last_block);
- last_block = malloc (memory_rqd);
+ last_block = g_try_malloc (memory_rqd);
if (last_block == NULL)
{
- fprintf (stderr, "render_rectangle: cannot malloc %d bytes.\n",
- memory_rqd);
+ g_printerr ("render_rectangle: cannot malloc %d bytes.\n",
+ memory_rqd);
exit (1);
}
last_block_size = memory_rqd;
---
From 5481b110f955213cca2e893f71b275a19f2530f7 Mon Sep 17 00:00:00 2001
From: Jacob Boerema <jgboerema@gmail.com>
Date: Mon, 31 Oct 2022 14:22:44 -0400
Subject: [PATCH] plug-ins: fix crash when reading corrupt flame settings file
Thanks to a report by Stefan Cornelius, we became aware that the flame
plug-in does no checking for correct input when loading a pre-saved
settings file.
I reworked the parser to read one or more values based on the type of
token, making sure we also don't read past the end of our token buffer.
All int values have a min and max value set. If any unexpected input is
encountered, we will give a warning.
(cherry picked from commit 89c83ef4c7a83bca5ca40f79d638ff365eb61464)
---
plug-ins/flame/libifs.c | 341 +++++++++++++++++++++++++++++-----------
1 file changed, 251 insertions(+), 90 deletions(-)
diff --git a/plug-ins/flame/libifs.c b/plug-ins/flame/libifs.c
index 7f158c853f..16259d46a5 100644
--- a/plug-ins/flame/libifs.c
+++ b/plug-ins/flame/libifs.c
@@ -911,8 +911,6 @@ compare_xforms (const void *va,
/*
* given a pointer to a string SS, fill fields of a control point CP.
- * return a pointer to the first unused char in SS. totally barfucious,
- * must integrate with tcl soon...
*/
void
@@ -921,10 +919,9 @@ parse_control_point (char **ss,
{
char *argv[MAXARGS];
int argc, i, j;
- int set_cm = 0, set_image_size = 0, set_nbatches = 0, set_white_level = 0, set_cmap_inter = 0;
- int set_spatial_oversample = 0;
- double *slot = NULL, xf, cm, t, nbatches, white_level, spatial_oversample, cmap_inter;
- double image_size[2];
+ gint64 xf_index = 0;
+ gint parse_errors = 0;
+ double *slot = NULL, t;
for (i = 0; i < NXFORMS; i++)
{
@@ -949,94 +946,258 @@ parse_control_point (char **ss,
}
tokenize (ss, argv, &argc);
- for (i = 0; i < argc; i++)
+
+ i = 0;
+ while (i < argc)
{
- if (streql("xform", argv[i]))
- slot = &xf;
- else if (streql("time", argv[i]))
- slot = &cp->time;
- else if (streql("brightness", argv[i]))
- slot = &cp->brightness;
- else if (streql("contrast", argv[i]))
- slot = &cp->contrast;
- else if (streql("gamma", argv[i]))
- slot = &cp->gamma;
- else if (streql("zoom", argv[i]))
- slot = &cp->zoom;
- else if (streql("image_size", argv[i]))
- {
- slot = image_size;
- set_image_size = 1;
- }
- else if (streql("center", argv[i]))
- slot = cp->center;
- else if (streql("pulse", argv[i]))
- slot = (double *) cp->pulse;
- else if (streql("wiggle", argv[i]))
- slot = (double *) cp->wiggle;
- else if (streql("pixels_per_unit", argv[i]))
- slot = &cp->pixels_per_unit;
- else if (streql("spatial_filter_radius", argv[i]))
- slot = &cp->spatial_filter_radius;
- else if (streql("sample_density", argv[i]))
- slot = &cp->sample_density;
- else if (streql("nbatches", argv[i]))
- {
- slot = &nbatches;
- set_nbatches = 1;
- }
- else if (streql("white_level", argv[i]))
- {
- slot = &white_level;
- set_white_level = 1;
- }
- else if (streql("spatial_oversample", argv[i]))
- {
- slot = &spatial_oversample;
- set_spatial_oversample = 1;
- }
- else if (streql("cmap", argv[i]))
- {
- slot = &cm;
- set_cm = 1;
- }
- else if (streql("density", argv[i]))
- slot = &cp->xform[(int)xf].density;
- else if (streql("color", argv[i]))
- slot = &cp->xform[(int)xf].color;
- else if (streql("coefs", argv[i]))
- {
- slot = cp->xform[(int)xf].c[0];
- cp->xform[(int)xf].density = 1.0;
- }
- else if (streql("var", argv[i]))
- slot = cp->xform[(int)xf].var;
- else if (streql("cmap_inter", argv[i]))
- {
- slot = &cmap_inter;
- set_cmap_inter = 1;
+ gint itoken;
+
+ itoken = i;
+ if (i < argc)
+ {
+ /* First value belonging to token. */
+ i++;
}
else
- *slot++ = g_strtod(argv[i], NULL);
- }
- if (set_cm)
- {
- cp->cmap_index = (int) cm;
- get_cmap(cp->cmap_index, cp->cmap, 256);
- }
- if (set_image_size)
- {
- cp->width = (int) image_size[0];
- cp->height = (int) image_size[1];
+ {
+ g_printerr ("Not enough parameters. File may be corrupt!\n");
+ parse_errors++;
+ break;
+ }
+
+ if (streql ("xform", argv[itoken]))
+ {
+ if (! g_ascii_string_to_signed (argv[i++], 10, 0, NXFORMS-1, &xf_index, NULL))
+ {
+ g_printerr ("Invalid xform index '%s'\n", argv[i-1]);
+ parse_errors++;
+ xf_index = 0;
+ }
+ }
+ else if (streql ("density", argv[itoken]))
+ {
+ cp->xform[xf_index].density = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("color", argv[itoken]))
+ {
+ cp->xform[xf_index].color = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("coefs", argv[itoken]))
+ {
+ /* We need 6 coef values and we know are at the first */
+ if (i + 5 >= argc)
+ {
+ g_printerr ("Not enough parameters. File may be corrupt!\n");
+ parse_errors++;
+ break;
+ }
+ slot = cp->xform[xf_index].c[0];
+ for (j = 0; j < 6; j++)
+ {
+ *slot++ = g_strtod (argv[i++], NULL);
+ }
+ cp->xform[xf_index].density = 1.0;
+ }
+ else if (streql ("var", argv[itoken]))
+ {
+ /* We need NVARS var values and we know are at the first */
+ if (i + NVARS > argc)
+ {
+ g_printerr ("Not enough parameters. File may be corrupt!\n");
+ parse_errors++;
+ break;
+ }
+ slot = cp->xform[xf_index].var;
+ for (j = 0; j < NVARS; j++)
+ {
+ *slot++ = g_strtod (argv[i++], NULL);
+ }
+ }
+ else if (streql ("time", argv[itoken]))
+ {
+ cp->time = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("brightness", argv[itoken]))
+ {
+ cp->brightness = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("contrast", argv[itoken]))
+ {
+ cp->contrast = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("gamma", argv[itoken]))
+ {
+ cp->gamma = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("zoom", argv[itoken]))
+ {
+ cp->zoom = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("image_size", argv[itoken]))
+ {
+ gint64 w, h;
+
+ /* We need 2 values and we know are at the first */
+ if (i + 1 >= argc)
+ {
+ g_printerr ("Not enough parameters. File may be corrupt!\n");
+ parse_errors++;
+ break;
+ }
+ if (! g_ascii_string_to_signed (argv[i++], 10, 1, GIMP_MAX_IMAGE_SIZE, &w, NULL))
+ {
+ g_printerr ("Ignoring invalid image width '%s'\n", argv[i-1]);
+ parse_errors++;
+ }
+ else if (! g_ascii_string_to_signed (argv[i++], 10, 1, GIMP_MAX_IMAGE_SIZE, &h, NULL))
+ {
+ g_printerr ("Ignoring invalid image_size heigth '%s'\n", argv[i-1]);
+ parse_errors++;
+ }
+ else
+ {
+ cp->width = w;
+ cp->height = h;
+ }
+ }
+ else if (streql ("center", argv[itoken]))
+ {
+ /* We need 2 values and we know are at the first */
+ if (i + 1 >= argc)
+ {
+ g_printerr ("Not enough parameters. File may be corrupt!\n");
+ parse_errors++;
+ break;
+ }
+ cp->center[0] = g_strtod (argv[i++], NULL);
+ cp->center[1] = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("pixels_per_unit", argv[itoken]))
+ {
+ cp->pixels_per_unit = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("pulse", argv[itoken]))
+ {
+ /* We need 4 values and we know are at the first */
+ if (i + 3 >= argc)
+ {
+ g_printerr ("Not enough parameters. File may be corrupt!\n");
+ parse_errors++;
+ break;
+ }
+ slot = &cp->pulse[0][0];
+ for (j = 0; j < 4; j++)
+ {
+ *slot++ = g_strtod (argv[i++], NULL);
+ }
+ }
+ else if (streql ("wiggle", argv[itoken]))
+ {
+ /* We need 4 values and we know are at the first */
+ if (i + 3 >= argc)
+ {
+ g_printerr ("Not enough parameters. File may be corrupt!\n");
+ parse_errors++;
+ break;
+ }
+ slot = &cp->wiggle[0][0];
+ for (j = 0; j < 4; j++)
+ {
+ *slot++ = g_strtod (argv[i++], NULL);
+ }
+ }
+ else if (streql ("spatial_oversample", argv[itoken]))
+ {
+ gint64 oversample;
+
+ /* Values in the gui seem to be between 1 and 4 */
+ if (! g_ascii_string_to_signed (argv[i++], 10, 1, 4, &oversample, NULL))
+ {
+ g_printerr ("Ignoring invalid spatial oversample value '%s'\n", argv[i-1]);
+ parse_errors++;
+ }
+ else
+ {
+ cp->spatial_oversample = oversample;
+ }
+ }
+ else if (streql ("spatial_filter_radius", argv[itoken]))
+ {
+ cp->spatial_filter_radius = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("sample_density", argv[itoken]))
+ {
+ cp->sample_density = g_strtod (argv[i++], NULL);
+ }
+ else if (streql ("nbatches", argv[itoken]))
+ {
+ gint64 nbatches;
+
+ /* Not sure what the maximum should be. It always seems to be set to 1. */
+ if (! g_ascii_string_to_signed (argv[i++], 10, 0, 2, &nbatches, NULL))
+ {
+ g_printerr ("Ignoring invalid nbatches value '%s'\n", argv[i-1]);
+ parse_errors++;
+ }
+ else
+ {
+ cp->nbatches = nbatches;
+ }
+ }
+ else if (streql ("white_level", argv[itoken]))
+ {
+ gint64 wl;
+
+ if (! g_ascii_string_to_signed (argv[i++], 10, 0, 255, &wl, NULL))
+ {
+ g_printerr ("Ignoring invalid white level value '%s'\n", argv[i-1]);
+ parse_errors++;
+ }
+ else
+ {
+ cp->white_level = wl;
+ }
+ }
+ else if (streql ("cmap", argv[itoken]))
+ {
+ gint64 cmi;
+
+ /* -1 = random */
+ if (! g_ascii_string_to_signed (argv[i++], 10, -1, 255, &cmi, NULL))
+ {
+ g_printerr ("Ignoring invalid color map value '%s'\n", argv[i-1]);
+ parse_errors++;
+ }
+ else
+ {
+ cp->cmap_index = cmi;
+ }
+ }
+ else if (streql ("cmap_inter", argv[itoken]))
+ {
+ gint64 cmi;
+
+ /* 0 or 1 */
+ if (! g_ascii_string_to_signed (argv[i++], 10, 0, 1, &cmi, NULL))
+ {
+ g_printerr ("Ignoring invalid color interpolate value '%s'\n", argv[i-1]);
+ parse_errors++;
+ }
+ else
+ {
+ cp->cmap_inter = cmi;
+ }
+ }
+ else
+ {
+ g_printerr ("Invalid token '%s'. File may be corrupt!\n", argv[itoken]);
+ parse_errors++;
+ }
}
- if (set_cmap_inter)
- cp->cmap_inter = (int) cmap_inter;
- if (set_nbatches)
- cp->nbatches = (int) nbatches;
- if (set_spatial_oversample)
- cp->spatial_oversample = (int) spatial_oversample;
- if (set_white_level)
- cp->white_level = (int) white_level;
+
+ if (parse_errors > 0)
+ g_warning ("Input file contains %d errors. File may be corrupt!", parse_errors);
+
for (i = 0; i < NXFORMS; i++)
{
t = 0.0;
--
From b36cc3ab3f0440edc611f35cc16442839526aa09 Mon Sep 17 00:00:00 2001
From: Jacob Boerema <jgboerema@gmail.com>
Date: Mon, 31 Oct 2022 14:00:54 -0400
Subject: [PATCH] plug-ins: fix missing input buffer length checking in flame
The flame plug-in can read stored settings from a file. The expected
input is that a ; signifies the end of input.
However, with user input we cannot depend on this to be true, so we need
to make sure that we do not read past the end of our input buffer.
To do so, we add a length check.
(cherry picked from commit 536c7cbc4b7c5fe5e1d03ac249d74f851a0e2e87)
---
plug-ins/flame/libifs.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/plug-ins/flame/libifs.c b/plug-ins/flame/libifs.c
index a4af48bb30..7f158c853f 100644
--- a/plug-ins/flame/libifs.c
+++ b/plug-ins/flame/libifs.c
@@ -842,10 +842,12 @@ tokenize (char **ss,
char *argv[],
int *argc)
{
- char *s = *ss;
- int i = 0, state = 0;
+ char *s = *ss;
+ int i = 0, state = 0;
+ gint len = 0;
- while (*s != ';')
+ len = strlen (s);
+ while (*s != ';' && len > 0)
{
char c = *s;
switch (state)
@@ -870,6 +872,7 @@ tokenize (char **ss,
state = 0;
}
s++;
+ len--;
}
*s = 0;
*ss = s + 1;
--
From b3186d72ee559b8ba78900acba134967338bf642 Mon Sep 17 00:00:00 2001
From: Jacob Boerema <jgboerema@gmail.com>
Date: Mon, 31 Oct 2022 13:57:14 -0400
Subject: [PATCH] plug-ins: fix failure to load flame saved settings from file
We were using the plug-in name with underscores, which is incorrect.
Since nobody ever complained about this, this doesn't seem to be used
very often.
We will use the const that defines the plug-in name instead.
(cherry picked from commit 193596397e6671ccb3684683adf1d2af48c14d05)
---
plug-ins/flame/flame.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/plug-ins/flame/flame.c b/plug-ins/flame/flame.c
index e7480cba1c..f2e3898f65 100644
--- a/plug-ins/flame/flame.c
+++ b/plug-ins/flame/flame.c
@@ -436,7 +436,7 @@ file_response_callback (GtkFileChooser *chooser,
fclose (f);
/* i want to update the existing dialogue, but it's
too painful */
- gimp_set_data ("plug_in_flame", &config, sizeof (config));
+ gimp_set_data (PLUG_IN_PROC, &config, sizeof (config));
/* gtk_widget_destroy(dialog); */
set_flame_preview ();
set_edit_preview ();
--
diff -urNp a/plug-ins/flame/libifs.c b/plug-ins/flame/libifs.c
--- a/plug-ins/flame/libifs.c 2023-01-11 12:37:26.893377867 +0100
+++ b/plug-ins/flame/libifs.c 2023-01-11 12:38:31.093911423 +0100
@@ -865,15 +865,18 @@ tokenize (char **ss,
i++;
state = 1;
}
+ break;
case 1:
if (g_ascii_isspace (c))
{
*s = 0;
state = 0;
}
+ break;
case 2:
if (c == '\n')
state = 0;
+ break;
}
s++;
len--;