| From 65ac6cda675fafd57bc182175f685e5d8c1a9cc9 Mon Sep 17 00:00:00 2001 |
| From: Nils Philippsen <nils@redhat.com> |
| 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 <nils@redhat.com> |
| Date: Fri Jul 13 15:47:16 2012 +0200 |
| |
| file-cel: close file on error |
| |
| commit ec3f1fe7586527ea7e2735b5c8548b925f622d5b |
| Author: Nils Philippsen <nils@redhat.com> |
| 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 <nils@redhat.com> |
| 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 <nils@redhat.com> |
| 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 <nils@redhat.com> |
| 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 |
| |