From: Michael Gilbert Date: Mon, 9 Sep 2013 17:34:32 +0200 Subject: Fix_CVE-2012-4433 Multiple buffer overflow issues. Closes: #692435 --- operations/external/ppm-load.c | 62 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/operations/external/ppm-load.c b/operations/external/ppm-load.c index efe6d56..465096d 100644 --- a/operations/external/ppm-load.c +++ b/operations/external/ppm-load.c @@ -36,6 +36,7 @@ gegl_chant_file_path (path, _("File"), "", _("Path of file to load.")) #include "gegl-chant.h" #include #include +#include typedef enum { PIXMAP_ASCII = 51, @@ -44,8 +45,8 @@ typedef enum { typedef struct { map_type type; - gint width; - gint height; + glong width; + glong height; gsize numsamples; /* width * height * channels */ gsize bpc; /* bytes per channel */ guchar *data; @@ -82,12 +83,33 @@ ppm_load_read_header(FILE *fp, } /* Get Width and Height */ - img->width = strtol (header,&ptr,0); - img->height = atoi (ptr); - img->numsamples = img->width * img->height * CHANNEL_COUNT; + errno = 0; + img->width = strtol (header,&ptr,10); + if (errno) + { + g_warning ("Error reading width: %s", strerror(errno)); + return FALSE; + } + else if (img->width < 0) + { + g_warning ("Error: width is negative"); + return FALSE; + } + + img->height = strtol (ptr,&ptr,10); + if (errno) + { + g_warning ("Error reading height: %s", strerror(errno)); + return FALSE; + } + else if (img->width < 0) + { + g_warning ("Error: height is negative"); + return FALSE; + } fgets (header,MAX_CHARS_IN_ROW,fp); - maxval = strtol (header,&ptr,0); + maxval = strtol (header,&ptr,10); if ((maxval != 255) && (maxval != 65535)) { @@ -109,6 +131,16 @@ ppm_load_read_header(FILE *fp, g_warning ("%s: Programmer stupidity error", G_STRLOC); } + /* Later on, img->numsamples is multiplied with img->bpc to allocate + * memory. Ensure it doesn't overflow. */ + if (!img->width || !img->height || + G_MAXSIZE / img->width / img->height / CHANNEL_COUNT < img->bpc) + { + g_warning ("Illegal width/height: %ld/%ld", img->width, img->height); + return FALSE; + } + img->numsamples = img->width * img->height * CHANNEL_COUNT; + return TRUE; } @@ -229,12 +261,24 @@ process (GeglOperation *operation, if (!ppm_load_read_header (fp, &img)) goto out; - rect.height = img.height; - rect.width = img.width; - /* Allocating Array Size */ + + /* Should use g_try_malloc(), but this causes crashes elsewhere because the + * error signalled by returning FALSE isn't properly acted upon. Therefore + * g_malloc() is used here which aborts if the requested memory size can't be + * allocated causing a controlled crash. */ img.data = (guchar*) g_malloc (img.numsamples * img.bpc); + /* No-op without g_try_malloc(), see above. */ + if (! img.data) + { + g_warning ("Couldn't allocate %" G_GSIZE_FORMAT " bytes, giving up.", ((gsize)img.numsamples * img.bpc)); + goto out; + } + + rect.height = img.height; + rect.width = img.width; + switch (img.bpc) { case 1: ovic Courtès