d8fab0
From ffa77a246652c7e706d690682fe659f50fbe5656 Mon Sep 17 00:00:00 2001
d8fab0
From: Nils Philippsen <nils@redhat.com>
d8fab0
Date: Mon, 1 Jul 2013 12:03:51 +0200
d8fab0
Subject: [PATCH] patch: CVE-2012-4433
d8fab0
d8fab0
Squashed commit of the following:
d8fab0
d8fab0
commit 2a9071e2dc4cfe1aaa7a726805985281936f9874
d8fab0
Author: Nils Philippsen <nils@redhat.com>
d8fab0
Date:   Tue Oct 16 16:57:37 2012 +0200
d8fab0
d8fab0
    ppm-load: bring comment in line with reality
d8fab0
d8fab0
    (cherry picked from commit 6975a9cfeaf0698b42ac81b1c2f00d13c8755453)
d8fab0
d8fab0
commit 8bb88ebf78e54837322d3be74688f98800e9f33a
d8fab0
Author: Nils Philippsen <nils@redhat.com>
d8fab0
Date:   Tue Oct 16 16:56:40 2012 +0200
d8fab0
d8fab0
    ppm-load: CVE-2012-4433: add plausibility checks for header fields
d8fab0
d8fab0
    Refuse values that are non-decimal, negative or overflow the target
d8fab0
    type.
d8fab0
d8fab0
    (cherry picked from commit 4757cdf73d3675478d645a3ec8250ba02168a230)
d8fab0
d8fab0
commit 2b099886969bf055a8635d06a4d89f20fed1ee42
d8fab0
Author: Nils Philippsen <nils@redhat.com>
d8fab0
Date:   Tue Oct 16 16:58:27 2012 +0200
d8fab0
d8fab0
    ppm-load: CVE-2012-4433: don't overflow memory allocation
d8fab0
d8fab0
    Carefully selected width/height values could cause the size of a later
d8fab0
    allocation to overflow, resulting in a buffer much too small to store
d8fab0
    the data which would then written beyond its end.
d8fab0
d8fab0
    (cherry picked from commit 1e92e5235ded0415d555aa86066b8e4041ee5a53)
d8fab0
---
d8fab0
 operations/external/ppm-load.c | 64 +++++++++++++++++++++++++++++++++++-------
d8fab0
 1 file changed, 54 insertions(+), 10 deletions(-)
d8fab0
d8fab0
diff --git a/operations/external/ppm-load.c b/operations/external/ppm-load.c
d8fab0
index efe6d56..e22521c 100644
d8fab0
--- a/operations/external/ppm-load.c
d8fab0
+++ b/operations/external/ppm-load.c
d8fab0
@@ -36,6 +36,7 @@ gegl_chant_file_path (path, _("File"), "", _("Path of file to load."))
d8fab0
 #include "gegl-chant.h"
d8fab0
 #include <stdio.h>
d8fab0
 #include <stdlib.h>
d8fab0
+#include <errno.h>
d8fab0
 
d8fab0
 typedef enum {
d8fab0
   PIXMAP_ASCII  = 51,
d8fab0
@@ -44,8 +45,8 @@ typedef enum {
d8fab0
 
d8fab0
 typedef struct {
d8fab0
 	map_type   type;
d8fab0
-	gint       width;
d8fab0
-	gint       height;
d8fab0
+	glong      width;
d8fab0
+	glong      height;
d8fab0
         gsize      numsamples; /* width * height * channels */
d8fab0
         gsize      bpc;        /* bytes per channel */
d8fab0
 	guchar    *data;
d8fab0
@@ -61,7 +62,7 @@ ppm_load_read_header(FILE       *fp,
d8fab0
     gchar  header[MAX_CHARS_IN_ROW];
d8fab0
     gint   maxval;
d8fab0
 
d8fab0
-    /* Check the PPM file Type P2 or P5 */
d8fab0
+    /* Check the PPM file Type P3 or P6 */
d8fab0
     fgets (header,MAX_CHARS_IN_ROW,fp);
d8fab0
 
d8fab0
     if (header[0] != ASCII_P ||
d8fab0
@@ -82,12 +83,33 @@ ppm_load_read_header(FILE       *fp,
d8fab0
       }
d8fab0
 
d8fab0
     /* Get Width and Height */
d8fab0
-    img->width  = strtol (header,&ptr,0);
d8fab0
-    img->height = atoi (ptr);
d8fab0
-    img->numsamples = img->width * img->height * CHANNEL_COUNT;
d8fab0
+    errno = 0;
d8fab0
+    img->width  = strtol (header,&ptr,10);
d8fab0
+    if (errno)
d8fab0
+      {
d8fab0
+        g_warning ("Error reading width: %s", strerror(errno));
d8fab0
+        return FALSE;
d8fab0
+      }
d8fab0
+    else if (img->width < 0)
d8fab0
+      {
d8fab0
+        g_warning ("Error: width is negative");
d8fab0
+        return FALSE;
d8fab0
+      }
d8fab0
+
d8fab0
+    img->height = strtol (ptr,&ptr,10);
d8fab0
+    if (errno)
d8fab0
+      {
d8fab0
+        g_warning ("Error reading height: %s", strerror(errno));
d8fab0
+        return FALSE;
d8fab0
+      }
d8fab0
+    else if (img->width < 0)
d8fab0
+      {
d8fab0
+        g_warning ("Error: height is negative");
d8fab0
+        return FALSE;
d8fab0
+      }
d8fab0
 
d8fab0
     fgets (header,MAX_CHARS_IN_ROW,fp);
d8fab0
-    maxval = strtol (header,&ptr,0);
d8fab0
+    maxval = strtol (header,&ptr,10);
d8fab0
 
d8fab0
     if ((maxval != 255) && (maxval != 65535))
d8fab0
       {
d8fab0
@@ -109,6 +131,16 @@ ppm_load_read_header(FILE       *fp,
d8fab0
       g_warning ("%s: Programmer stupidity error", G_STRLOC);
d8fab0
     }
d8fab0
 
d8fab0
+    /* Later on, img->numsamples is multiplied with img->bpc to allocate
d8fab0
+     * memory. Ensure it doesn't overflow. */
d8fab0
+    if (!img->width || !img->height ||
d8fab0
+        G_MAXSIZE / img->width / img->height / CHANNEL_COUNT < img->bpc)
d8fab0
+      {
d8fab0
+        g_warning ("Illegal width/height: %ld/%ld", img->width, img->height);
d8fab0
+        return FALSE;
d8fab0
+      }
d8fab0
+    img->numsamples = img->width * img->height * CHANNEL_COUNT;
d8fab0
+
d8fab0
     return TRUE;
d8fab0
 }
d8fab0
 
d8fab0
@@ -229,12 +261,24 @@ process (GeglOperation       *operation,
d8fab0
   if (!ppm_load_read_header (fp, &img))
d8fab0
     goto out;
d8fab0
 
d8fab0
-  rect.height = img.height;
d8fab0
-  rect.width = img.width;
d8fab0
-
d8fab0
   /* Allocating Array Size */
d8fab0
+
d8fab0
+  /* Should use g_try_malloc(), but this causes crashes elsewhere because the
d8fab0
+   * error signalled by returning FALSE isn't properly acted upon. Therefore
d8fab0
+   * g_malloc() is used here which aborts if the requested memory size can't be
d8fab0
+   * allocated causing a controlled crash. */
d8fab0
   img.data = (guchar*) g_malloc (img.numsamples * img.bpc);
d8fab0
 
d8fab0
+  /* No-op without g_try_malloc(), see above. */
d8fab0
+  if (! img.data)
d8fab0
+    {
d8fab0
+      g_warning ("Couldn't allocate %" G_GSIZE_FORMAT " bytes, giving up.", ((gsize)img.numsamples * img.bpc));
d8fab0
+      goto out;
d8fab0
+    }
d8fab0
+
d8fab0
+  rect.height = img.height;
d8fab0
+  rect.width = img.width;
d8fab0
+
d8fab0
   switch (img.bpc)
d8fab0
     {
d8fab0
     case 1:
d8fab0
-- 
d8fab0
1.8.3.1
d8fab0