From 993bbd354ea10e346c18899866b0d3683ddbfdb4 Mon Sep 17 00:00:00 2001 From: Ell Date: Wed, 20 Jun 2018 14:47:15 -0400 Subject: [PATCH] Issue #1682 - Segfault when starting GIMP, due to empty data files Use gimp_input_data_stream_read_line_always(), instead of g_input_data_stream_read_line(), in a bunch of places that don't expect EOF. If we don't do that, the code assumes the GError parameter is set by the function and returns an error indication, causing the caller to segfault when it tries to access error->message. Instead, we now process an empty line when EOF is reached, which is caught by the normal parsing logic. Additionally: - Use gimp_ascii_strto[id]() when loading gradients, generated brushes, and palettes, to improve error checking for invalid numeric input. - Improve gradient-segment endpoint consistency check. - Allow loading palette files with 0 colors. They can be created during the session, so we might as well successfully load them. --- app/core/gimpbrushgenerated-load.c | 91 +++++++++++----- app/core/gimpgradient-load.c | 166 +++++++++++++---------------- app/core/gimppalette-load.c | 52 +++++---- app/operations/gimpcurvesconfig.c | 5 +- app/operations/gimplevelsconfig.c | 9 +- 5 files changed, 175 insertions(+), 148 deletions(-) diff --git a/app/core/gimpbrushgenerated-load.c b/app/core/gimpbrushgenerated-load.c index 2d03a028b0..2e122f49bb 100644 --- a/app/core/gimpbrushgenerated-load.c +++ b/app/core/gimpbrushgenerated-load.c @@ -28,6 +28,7 @@ #include "core-types.h" +#include "gimp-utils.h" #include "gimpbrushgenerated.h" #include "gimpbrushgenerated-load.h" @@ -64,8 +65,8 @@ gimp_brush_generated_load (GimpContext *context, /* make sure the file we are reading is the right type */ linenum = 1; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; @@ -82,8 +83,8 @@ gimp_brush_generated_load (GimpContext *context, /* make sure we are reading a compatible version */ linenum++; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; @@ -107,8 +108,8 @@ gimp_brush_generated_load (GimpContext *context, /* read name */ linenum++; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; @@ -138,8 +139,8 @@ gimp_brush_generated_load (GimpContext *context, /* read shape */ linenum++; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; @@ -162,21 +163,34 @@ gimp_brush_generated_load (GimpContext *context, /* read brush spacing */ linenum++; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; - spacing = g_ascii_strtod (string, NULL); + if (! gimp_ascii_strtod (string, NULL, &spacing)) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Invalid brush spacing.")); + g_free (string); + goto failed; + } g_free (string); + /* read brush radius */ linenum++; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; - radius = g_ascii_strtod (string, NULL); + if (! gimp_ascii_strtod (string, NULL, &radius)) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Invalid brush radius.")); + g_free (string); + goto failed; + } g_free (string); if (have_shape) @@ -184,42 +198,67 @@ gimp_brush_generated_load (GimpContext *context, /* read number of spikes */ linenum++; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; - spikes = CLAMP (atoi (string), 2, 20); + if (! gimp_ascii_strtoi (string, NULL, 10, &spikes) || + spikes < 2 || spikes > 20) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Invalid brush spike count.")); + g_free (string); + goto failed; + } g_free (string); } /* read brush hardness */ linenum++; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; - hardness = g_ascii_strtod (string, NULL); + if (! gimp_ascii_strtod (string, NULL, &hardness)) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Invalid brush hardness.")); + g_free (string); + goto failed; + } g_free (string); /* read brush aspect_ratio */ linenum++; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; - aspect_ratio = g_ascii_strtod (string, NULL); + if (! gimp_ascii_strtod (string, NULL, &aspect_ratio)) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Invalid brush aspect ratio.")); + g_free (string); + goto failed; + } g_free (string); /* read brush angle */ linenum++; string_len = 256; - string = g_data_input_stream_read_line (data_input, &string_len, - NULL, error); + string = gimp_data_input_stream_read_line_always (data_input, &string_len, + NULL, error); if (! string) goto failed; - angle = g_ascii_strtod (string, NULL); + if (! gimp_ascii_strtod (string, NULL, &angle)) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Invalid brush angle.")); + g_free (string); + goto failed; + } g_free (string); g_object_unref (data_input); diff --git a/app/core/gimpgradient-load.c b/app/core/gimpgradient-load.c index cc23086ba0..f4129578ac 100644 --- a/app/core/gimpgradient-load.c +++ b/app/core/gimpgradient-load.c @@ -32,6 +32,7 @@ #include "config/gimpxmlparser.h" +#include "gimp-utils.h" #include "gimpgradient.h" #include "gimpgradient-load.h" @@ -61,8 +62,8 @@ gimp_gradient_load (GimpContext *context, linenum = 1; line_len = 1024; - line = g_data_input_stream_read_line (data_input, &line_len, - NULL, error); + line = gimp_data_input_stream_read_line_always (data_input, &line_len, + NULL, error); if (! line) goto failed; @@ -82,8 +83,8 @@ gimp_gradient_load (GimpContext *context, linenum++; line_len = 1024; - line = g_data_input_stream_read_line (data_input, &line_len, - NULL, error); + line = gimp_data_input_stream_read_line_always (data_input, &line_len, + NULL, error); if (! line) goto failed; @@ -100,8 +101,8 @@ gimp_gradient_load (GimpContext *context, linenum++; line_len = 1024; - line = g_data_input_stream_read_line (data_input, &line_len, - NULL, error); + line = gimp_data_input_stream_read_line_always (data_input, &line_len, + NULL, error); if (! line) goto failed; } @@ -128,6 +129,10 @@ gimp_gradient_load (GimpContext *context, { GimpGradientSegment *seg; gchar *end; + gint color; + gint type; + gint left_color_type; + gint right_color_type; seg = gimp_gradient_segment_new (); @@ -140,99 +145,80 @@ gimp_gradient_load (GimpContext *context, linenum++; line_len = 1024; - line = g_data_input_stream_read_line (data_input, &line_len, - NULL, error); + line = gimp_data_input_stream_read_line_always (data_input, &line_len, + NULL, error); if (! line) goto failed; - seg->left = g_ascii_strtod (line, &end); - if (end && errno != ERANGE) - seg->middle = g_ascii_strtod (end, &end); - if (end && errno != ERANGE) - seg->right = g_ascii_strtod (end, &end); + if (! gimp_ascii_strtod (line, &end, &seg->left) || + ! gimp_ascii_strtod (end, &end, &seg->middle) || + ! gimp_ascii_strtod (end, &end, &seg->right) || - if (end && errno != ERANGE) - seg->left_color.r = g_ascii_strtod (end, &end); - if (end && errno != ERANGE) - seg->left_color.g = g_ascii_strtod (end, &end); - if (end && errno != ERANGE) - seg->left_color.b = g_ascii_strtod (end, &end); - if (end && errno != ERANGE) - seg->left_color.a = g_ascii_strtod (end, &end); + ! gimp_ascii_strtod (end, &end, &seg->left_color.r) || + ! gimp_ascii_strtod (end, &end, &seg->left_color.g) || + ! gimp_ascii_strtod (end, &end, &seg->left_color.b) || + ! gimp_ascii_strtod (end, &end, &seg->left_color.a) || - if (end && errno != ERANGE) - seg->right_color.r = g_ascii_strtod (end, &end); - if (end && errno != ERANGE) - seg->right_color.g = g_ascii_strtod (end, &end); - if (end && errno != ERANGE) - seg->right_color.b = g_ascii_strtod (end, &end); - if (end && errno != ERANGE) - seg->right_color.a = g_ascii_strtod (end, &end); - - if (errno != ERANGE) + ! gimp_ascii_strtod (end, &end, &seg->right_color.r) || + ! gimp_ascii_strtod (end, &end, &seg->right_color.g) || + ! gimp_ascii_strtod (end, &end, &seg->right_color.b) || + ! gimp_ascii_strtod (end, &end, &seg->right_color.a)) { - gint color; - gint type; - gint left_color_type; - gint right_color_type; + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Corrupt segment %d."), i); + g_free (line); + goto failed; + } - switch (sscanf (end, "%d %d %d %d", - &type, &color, - &left_color_type, &right_color_type)) + switch (sscanf (end, "%d %d %d %d", + &type, &color, + &left_color_type, &right_color_type)) + { + case 4: + seg->left_color_type = (GimpGradientColor) left_color_type; + if (seg->left_color_type < GIMP_GRADIENT_COLOR_FIXED || + seg->left_color_type > GIMP_GRADIENT_COLOR_BACKGROUND_TRANSPARENT) { - case 4: - seg->left_color_type = (GimpGradientColor) left_color_type; - if (seg->left_color_type < GIMP_GRADIENT_COLOR_FIXED || - seg->left_color_type > GIMP_GRADIENT_COLOR_BACKGROUND_TRANSPARENT) - { - g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, - _("Corrupt segment %d."), i); - g_free (line); - goto failed; - } - - seg->right_color_type = (GimpGradientColor) right_color_type; - if (seg->right_color_type < GIMP_GRADIENT_COLOR_FIXED || - seg->right_color_type > GIMP_GRADIENT_COLOR_BACKGROUND_TRANSPARENT) - { - g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, - _("Corrupt segment %d."), i); - g_free (line); - goto failed; - } - /* fall thru */ - - case 2: - seg->type = (GimpGradientSegmentType) type; - if (seg->type < GIMP_GRADIENT_SEGMENT_LINEAR || - seg->type > GIMP_GRADIENT_SEGMENT_SPHERE_DECREASING) - { - g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, - _("Corrupt segment %d."), i); - g_free (line); - goto failed; - } - - seg->color = (GimpGradientSegmentColor) color; - if (seg->color < GIMP_GRADIENT_SEGMENT_RGB || - seg->color > GIMP_GRADIENT_SEGMENT_HSV_CW) - { - g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, - _("Corrupt segment %d."), i); - g_free (line); - goto failed; - } - break; - - default: g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, _("Corrupt segment %d."), i); g_free (line); goto failed; } - } - else - { + + seg->right_color_type = (GimpGradientColor) right_color_type; + if (seg->right_color_type < GIMP_GRADIENT_COLOR_FIXED || + seg->right_color_type > GIMP_GRADIENT_COLOR_BACKGROUND_TRANSPARENT) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Corrupt segment %d."), i); + g_free (line); + goto failed; + } + /* fall thru */ + + case 2: + seg->type = (GimpGradientSegmentType) type; + if (seg->type < GIMP_GRADIENT_SEGMENT_LINEAR || + seg->type > GIMP_GRADIENT_SEGMENT_SPHERE_DECREASING) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Corrupt segment %d."), i); + g_free (line); + goto failed; + } + + seg->color = (GimpGradientSegmentColor) color; + if (seg->color < GIMP_GRADIENT_SEGMENT_RGB || + seg->color > GIMP_GRADIENT_SEGMENT_HSV_CW) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Corrupt segment %d."), i); + g_free (line); + goto failed; + } + break; + + default: g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, _("Corrupt segment %d."), i); g_free (line); @@ -241,8 +227,10 @@ gimp_gradient_load (GimpContext *context, g_free (line); - if (( prev && (prev->right < seg->left)) || - (! prev && (0.0 < seg->left))) + if (seg->left > seg->middle || + seg->middle > seg->right || + ( prev && (prev->right != seg->left)) || + (! prev && (0.0 != seg->left))) { g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, _("Segments do not span the range 0-1.")); @@ -252,7 +240,7 @@ gimp_gradient_load (GimpContext *context, prev = seg; } - if (prev->right < 1.0) + if (prev->right != 1.0) { g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, _("Segments do not span the range 0-1.")); diff --git a/app/core/gimppalette-load.c b/app/core/gimppalette-load.c index c2af11fc62..83de26cd6b 100644 --- a/app/core/gimppalette-load.c +++ b/app/core/gimppalette-load.c @@ -28,6 +28,7 @@ #include "core-types.h" +#include "gimp-utils.h" #include "gimppalette.h" #include "gimppalette-load.h" @@ -59,8 +60,8 @@ gimp_palette_load (GimpContext *context, linenum = 1; str_len = 1024; - str = g_data_input_stream_read_line (data_input, &str_len, - NULL, error); + str = gimp_data_input_stream_read_line_always (data_input, &str_len, + NULL, error); if (! str) goto failed; @@ -80,8 +81,8 @@ gimp_palette_load (GimpContext *context, linenum++; str_len = 1024; - str = g_data_input_stream_read_line (data_input, &str_len, - NULL, error); + str = gimp_data_input_stream_read_line_always (data_input, &str_len, + NULL, error); if (! str) goto failed; @@ -97,8 +98,8 @@ gimp_palette_load (GimpContext *context, linenum++; str_len = 1024; - str = g_data_input_stream_read_line (data_input, &str_len, - NULL, error); + str = gimp_data_input_stream_read_line_always (data_input, &str_len, + NULL, error); if (! str) goto failed; @@ -106,7 +107,14 @@ gimp_palette_load (GimpContext *context, { gint columns; - columns = atoi (g_strstrip (str + strlen ("Columns: "))); + if (! gimp_ascii_strtoi (g_strstrip (str + strlen ("Columns: ")), + NULL, 10, &columns)) + { + g_set_error (error, GIMP_DATA_ERROR, GIMP_DATA_ERROR_READ, + _("Invalid column count.")); + g_free (str); + goto failed; + } if (columns < 0 || columns > 256) { @@ -122,8 +130,8 @@ gimp_palette_load (GimpContext *context, linenum++; str_len = 1024; - str = g_data_input_stream_read_line (data_input, &str_len, - NULL, error); + str = gimp_data_input_stream_read_line_always (data_input, &str_len, + NULL, error); if (! str) goto failed; } @@ -138,7 +146,7 @@ gimp_palette_load (GimpContext *context, { GError *my_error = NULL; - if (str[0] != '#' && str[0] != '\n') + if (str[0] != '#' && str[0] != '\0') { tok = strtok (str, " \t"); if (tok) @@ -196,24 +204,14 @@ gimp_palette_load (GimpContext *context, str_len = 1024; str = g_data_input_stream_read_line (data_input, &str_len, NULL, &my_error); - if (! str) + if (! str && my_error) { - if (! palette->colors) - { - g_propagate_error (error, my_error); - goto failed; - } - - if (my_error) - { - g_message (_("Reading palette file '%s': " - "Read %d colors from truncated file: %s"), - gimp_file_get_utf8_name (file), - g_list_length (palette->colors), - my_error->message); - g_clear_error (&my_error); - break; - } + g_message (_("Reading palette file '%s': " + "Read %d colors from truncated file: %s"), + gimp_file_get_utf8_name (file), + g_list_length (palette->colors), + my_error->message); + g_clear_error (&my_error); } } diff --git a/app/operations/gimpcurvesconfig.c b/app/operations/gimpcurvesconfig.c index 013bc5b584..349b6982cc 100644 --- a/app/operations/gimpcurvesconfig.c +++ b/app/operations/gimpcurvesconfig.c @@ -31,6 +31,7 @@ #include "operations-types.h" +#include "core/gimp-utils.h" #include "core/gimpcurve.h" #include "core/gimphistogram.h" @@ -537,8 +538,8 @@ gimp_curves_config_load_cruft (GimpCurvesConfig *config, data_input = g_data_input_stream_new (input); line_len = 64; - line = g_data_input_stream_read_line (data_input, &line_len, - NULL, error); + line = gimp_data_input_stream_read_line_always (data_input, &line_len, + NULL, error); if (! line) return FALSE; diff --git a/app/operations/gimplevelsconfig.c b/app/operations/gimplevelsconfig.c index 7f05aa2d39..e8d8561237 100644 --- a/app/operations/gimplevelsconfig.c +++ b/app/operations/gimplevelsconfig.c @@ -33,6 +33,7 @@ #include "operations-types.h" +#include "core/gimp-utils.h" #include "core/gimpcurve.h" #include "core/gimphistogram.h" @@ -831,8 +832,8 @@ gimp_levels_config_load_cruft (GimpLevelsConfig *config, data_input = g_data_input_stream_new (input); line_len = 64; - line = g_data_input_stream_read_line (data_input, &line_len, - NULL, error); + line = gimp_data_input_stream_read_line_always (data_input, &line_len, + NULL, error); if (! line) return FALSE; @@ -854,8 +855,8 @@ gimp_levels_config_load_cruft (GimpLevelsConfig *config, gint fields; line_len = 64; - line = g_data_input_stream_read_line (data_input, &line_len, - NULL, error); + line = gimp_data_input_stream_read_line_always (data_input, &line_len, + NULL, error); if (! line) { g_object_unref (data_input);