From 65ac6cda675fafd57bc182175f685e5d8c1a9cc9 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Mon, 20 Aug 2012 15:28:44 +0200 Subject: [PATCH] patch: CVE-2012-3403 Squashed commit of the following: commit d002e513039a9667a06d3e2ba180f9c18785cc5f Author: Nils Philippsen Date: Fri Jul 13 15:47:16 2012 +0200 file-cel: close file on error commit ec3f1fe7586527ea7e2735b5c8548b925f622d5b Author: Nils Philippsen Date: Fri Jul 13 15:33:27 2012 +0200 file-cel: use g_set_error() for errors instead of g_message() (cherry picked from commit 86f4cd39bd493c88a7a19b56d1827d8b911e07f6) Conflicts: plug-ins/common/file-cel.c commit 79bd89bc39195974d5cae2c2b06c829dd90c36ee Author: Nils Philippsen Date: Fri Jul 13 15:30:44 2012 +0200 file-cel: use statically allocated palette buffer (cherry picked from commit 69b98191cf315bcf0f7b8878896c01600e67c124) commit 52d85468980b5947cfd3e84f9a256769158210cc Author: Nils Philippsen Date: Fri Jul 13 15:20:06 2012 +0200 file-cel: validate header data (CVE-2012-3403) (cherry picked from commit b772d1b84c9272bb46ab9a21db4390e6263c9892) commit 62da97876070839097671e83eb8f5d408515396f Author: Nils Philippsen Date: Thu Jul 12 15:50:02 2012 +0200 file-cel: check fread()/g_fopen() return values and pass on errors (cherry picked from commit 797db58b94c64f418c35d38b7a608d933c8cebef) --- plug-ins/common/file-cel.c | 283 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 234 insertions(+), 49 deletions(-) diff --git a/plug-ins/common/file-cel.c b/plug-ins/common/file-cel.c index a94671c..3357561 100644 --- a/plug-ins/common/file-cel.c +++ b/plug-ins/common/file-cel.c @@ -44,8 +44,10 @@ static void run (const gchar *name, gint *nreturn_vals, GimpParam **return_vals); -static gint load_palette (FILE *fp, - guchar palette[]); +static gint load_palette (const gchar *file, + FILE *fp, + guchar palette[], + GError **error); static gint32 load_image (const gchar *file, const gchar *brief, GError **error); @@ -55,7 +57,8 @@ static gboolean save_image (const gchar *file, gint32 layer, GError **error); static void palette_dialog (const gchar *title); -static gboolean need_palette (const gchar *file); +static gboolean need_palette (const gchar *file, + GError **error); /* Globals... */ @@ -150,6 +153,7 @@ run (const gchar *name, gint32 image; GimpExportReturn export = GIMP_EXPORT_CANCEL; GError *error = NULL; + gint needs_palette = 0; run_mode = param[0].data.d_int32; @@ -187,20 +191,32 @@ run (const gchar *name, else if (run_mode == GIMP_RUN_INTERACTIVE) { /* Let user choose KCF palette (cancel ignores) */ - if (need_palette (param[1].data.d_string)) - palette_dialog (_("Load KISS Palette")); + needs_palette = need_palette (param[1].data.d_string, &error); - gimp_set_data (SAVE_PROC, palette_file, data_length); - } + if (! error) + { + if (needs_palette) + palette_dialog (_("Load KISS Palette")); - image = load_image (param[1].data.d_string, param[2].data.d_string, - &error); + gimp_set_data (SAVE_PROC, palette_file, data_length); + } + } - if (image != -1) + if (! error) { - *nreturn_vals = 2; - values[1].type = GIMP_PDB_IMAGE; - values[1].data.d_image = image; + image = load_image (param[1].data.d_string, param[2].data.d_string, + &error); + + if (image != -1) + { + *nreturn_vals = 2; + values[1].type = GIMP_PDB_IMAGE; + values[1].data.d_image = image; + } + else + { + status = GIMP_PDB_EXECUTION_ERROR; + } } else { @@ -263,18 +279,33 @@ run (const gchar *name, /* Peek into the file to determine whether we need a palette */ static gboolean -need_palette (const gchar *file) +need_palette (const gchar *file, + GError **error) { FILE *fp; guchar header[32]; + size_t n_read; fp = g_fopen (file, "rb"); - if (!fp) - return FALSE; + if (fp == NULL) + { + g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errno), + _("Could not open '%s' for reading: %s"), + gimp_filename_to_utf8 (file), g_strerror (errno)); + return FALSE; + } + + n_read = fread (header, 32, 1, fp); - fread (header, 32, 1, fp); fclose (fp); + if (n_read < 1) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("EOF or error while reading image header")); + return FALSE; + } + return (header[5] < 32); } @@ -286,11 +317,12 @@ load_image (const gchar *file, GError **error) { FILE *fp; /* Read file pointer */ - guchar header[32]; /* File header */ + guchar header[32], /* File header */ + file_mark, /* KiSS file type */ + bpp; /* Bits per pixel */ gint height, width, /* Dimensions of image */ offx, offy, /* Layer offets */ - colours, /* Number of colours */ - bpp; /* Bits per pixel */ + colours; /* Number of colours */ gint32 image, /* Image */ layer; /* Layer */ @@ -301,6 +333,7 @@ load_image (const gchar *file, GimpPixelRgn pixel_rgn; /* Pixel region for layer */ gint i, j, k; /* Counters */ + size_t n_read; /* Number of items read from file */ /* Open the file for reading */ @@ -319,7 +352,14 @@ load_image (const gchar *file, /* Get the image dimensions and create the image... */ - fread (header, 4, 1, fp); + n_read = fread (header, 4, 1, fp); + + if (n_read < 1) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("EOF or error while reading image header")); + return -1; + } if (strncmp ((const gchar *) header, "KiSS", 4)) { @@ -332,18 +372,53 @@ load_image (const gchar *file, } else { /* New-style image file, read full header */ - fread (header, 28, 1, fp); + n_read = fread (header, 28, 1, fp); + + if (n_read < 1) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("EOF or error while reading image header")); + return -1; + } + + file_mark = header[0]; + if (file_mark != 0x20 && file_mark != 0x21) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("is not a CEL image file")); + return -1; + } + bpp = header[1]; - if (bpp == 24) - colours = -1; - else - colours = (1 << header[1]); + switch (bpp) + { + case 4: + case 8: + case 32: + colours = (1 << bpp); + break; + default: + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("illegal bpp value in image: %hhu"), bpp); + return -1; + } + width = header[4] + (256 * header[5]); height = header[6] + (256 * header[7]); offx = header[8] + (256 * header[9]); offy = header[10] + (256 * header[11]); } + if ((width == 0) || (height == 0) || (width + offx > GIMP_MAX_IMAGE_SIZE) || + (height + offy > GIMP_MAX_IMAGE_SIZE)) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("illegal image dimensions: width: %d, horizontal offset: " + "%d, height: %d, vertical offset: %d"), + width, offx, height, offy); + return -1; + } + if (bpp == 32) image = gimp_image_new (width + offx, height + offy, GIMP_RGB); else @@ -351,7 +426,8 @@ load_image (const gchar *file, if (image == -1) { - g_message (_("Can't create a new image")); + g_set_error (error, 0, 0, _("Can't create a new image")); + fclose (fp); return -1; } @@ -383,7 +459,15 @@ load_image (const gchar *file, switch (bpp) { case 4: - fread (buffer, (width+1)/2, 1, fp); + n_read = fread (buffer, (width+1)/2, 1, fp); + + if (n_read < 1) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("EOF or error while reading image data")); + return -1; + } + for (j = 0, k = 0; j < width*2; j+= 4, ++k) { if (buffer[k] / 16 == 0) @@ -410,7 +494,15 @@ load_image (const gchar *file, break; case 8: - fread (buffer, width, 1, fp); + n_read = fread (buffer, width, 1, fp); + + if (n_read < 1) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("EOF or error while reading image data")); + return -1; + } + for (j = 0, k = 0; j < width*2; j+= 2, ++k) { if (buffer[k] == 0) @@ -427,7 +519,15 @@ load_image (const gchar *file, break; case 32: - fread (line, width*4, 1, fp); + n_read = fread (line, width*4, 1, fp); + + if (n_read < 1) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("EOF or error while reading image data")); + return -1; + } + /* The CEL file order is BGR so we need to swap B and R * to get the Gimp RGB order. */ @@ -440,7 +540,8 @@ load_image (const gchar *file, break; default: - g_message (_("Unsupported bit depth (%d)!"), bpp); + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("Unsupported bit depth (%d)!"), bpp); return -1; } @@ -457,7 +558,7 @@ load_image (const gchar *file, if (bpp != 32) { /* Use palette from file or otherwise default grey palette */ - palette = g_new (guchar, colours*3); + guchar palette[256*3]; /* Open the file for reading if user picked one */ if (palette_file == NULL) @@ -467,12 +568,23 @@ load_image (const gchar *file, else { fp = g_fopen (palette_file, "r"); + + if (fp == NULL) + { + g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errno), + _("Could not open '%s' for reading: %s"), + gimp_filename_to_utf8 (palette_file), + g_strerror (errno)); + return -1; + } } if (fp != NULL) { - colours = load_palette (fp, palette); + colours = load_palette (palette_file, fp, palette, error); fclose (fp); + if (colours < 0 || *error) + return -1; } else { @@ -483,10 +595,6 @@ load_image (const gchar *file, } gimp_image_set_colormap (image, palette + 3, colours - 1); - - /* Close palette file, give back allocated memory */ - - g_free (palette); } /* Now get everything redrawn and hand back the finished image */ @@ -498,32 +606,100 @@ load_image (const gchar *file, } static gint -load_palette (FILE *fp, - guchar palette[]) +load_palette (const gchar *file, + FILE *fp, + guchar palette[], + GError **error) { guchar header[32]; /* File header */ guchar buffer[2]; - int i, bpp, colours= 0; + guchar file_mark, bpp; + gint i, colours = 0; + size_t n_read; + + n_read = fread (header, 4, 1, fp); + + if (n_read < 1) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("'%s': EOF or error while reading palette header"), + gimp_filename_to_utf8 (file)); + return -1; + } - fread (header, 4, 1, fp); if (!strncmp ((const gchar *) header, "KiSS", 4)) { - fread (header+4, 28, 1, fp); + n_read = fread (header+4, 28, 1, fp); + + if (n_read < 1) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("'%s': EOF or error while reading palette header"), + gimp_filename_to_utf8 (file)); + return -1; + } + + file_mark = header[4]; + if (file_mark != 0x10) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("'%s': is not a KCF palette file"), + gimp_filename_to_utf8 (file)); + return -1; + } + bpp = header[5]; + if (bpp != 12 && bpp != 24) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("'%s': illegal bpp value in palette: %hhu"), + gimp_filename_to_utf8 (file), bpp); + return -1; + } + colours = header[8] + header[9] * 256; - if (bpp == 12) + if (colours != 16 && colours != 256) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("'%s': illegal number of colors: %u"), + gimp_filename_to_utf8 (file), colours); + return -1; + } + + switch (bpp) { + case 12: for (i = 0; i < colours; ++i) { - fread (buffer, 1, 2, fp); + n_read = fread (buffer, 1, 2, fp); + + if (n_read < 2) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("'%s': EOF or error while reading " + "palette data"), + gimp_filename_to_utf8 (file)); + return -1; + } + palette[i*3]= buffer[0] & 0xf0; palette[i*3+1]= (buffer[1] & 0x0f) * 16; palette[i*3+2]= (buffer[0] & 0x0f) * 16; } - } - else - { - fread (palette, colours, 3, fp); + break; + case 24: + n_read = fread (palette, colours, 3, fp); + + if (n_read < 3) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("'%s': EOF or error while reading palette data"), + gimp_filename_to_utf8 (file)); + return -1; + } + break; + default: + g_assert_not_reached (); } } else @@ -532,7 +708,16 @@ load_palette (FILE *fp, fseek (fp, 0, SEEK_SET); for (i= 0; i < colours; ++i) { - fread (buffer, 1, 2, fp); + n_read = fread (buffer, 1, 2, fp); + + if (n_read < 2) + { + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED, + _("'%s': EOF or error while reading palette data"), + gimp_filename_to_utf8 (file)); + return -1; + } + palette[i*3] = buffer[0] & 0xf0; palette[i*3+1] = (buffer[1] & 0x0f) * 16; palette[i*3+2] = (buffer[0] & 0x0f) * 16; -- 1.7.11.4