Attachment 633906 Details for Bug 856300 – Proposed patch against gegl git master

[patch] Proposed patch against gegl git master

gegl-master-ppm-segfault.patch (text/plain), 5.23 KB, created by Nils Philippsen on 2012-10-26 14:59:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nils Philippsen

Created: 2012-10-26 14:59:28 UTC

Size: 5.23 KB

patch

obsolete

>From 021add95ac3bcd7f60932c63c7c1ed5cec765c4d Mon Sep 17 00:00:00 2001
>From: Nils Philippsen <nils@redhat.com>
>Date: Tue, 16 Oct 2012 16:58:27 +0200
>Subject: [PATCH 1/3] ppm-load: CVE-2012-4433: don't overflow memory
> allocation
>
>Carefully selected width/height values could cause the size of a later
>allocation to overflow, resulting in a buffer much too small to store
>the data which would then written beyond its end.
>---
> operations/external/ppm-load.c | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
>diff --git a/operations/external/ppm-load.c b/operations/external/ppm-load.c
>index efe6d56..3d6bce7 100644
>--- a/operations/external/ppm-load.c
>+++ b/operations/external/ppm-load.c
>@@ -84,7 +84,6 @@ 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;
> 
>     fgets (header,MAX_CHARS_IN_ROW,fp);
>     maxval = strtol (header,&ptr,0);
>@@ -109,6 +108,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 +238,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:
>-- 
>1.7.11.7
>
>
>From 147388a43d1a67000a409163098abec30a4194c0 Mon Sep 17 00:00:00 2001
>From: Nils Philippsen <nils@redhat.com>
>Date: Tue, 16 Oct 2012 16:56:40 +0200
>Subject: [PATCH 2/3] ppm-load: CVE-2012-4433: add plausibility checks for
> header fields
>
>Refuse values that are non-decimal, negative or overflow the target
>type.
>---
> operations/external/ppm-load.c | 33 ++++++++++++++++++++++++++++-----
> 1 file changed, 28 insertions(+), 5 deletions(-)
>
>diff --git a/operations/external/ppm-load.c b/operations/external/ppm-load.c
>index 3d6bce7..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 <stdio.h>
> #include <stdlib.h>
>+#include <errno.h>
> 
> 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,11 +83,33 @@ ppm_load_read_header(FILE       *fp,
>       }
> 
>     /* Get Width and Height */
>-    img->width  = strtol (header,&ptr,0);
>-    img->height = atoi (ptr);
>+    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))
>       {
>-- 
>1.7.11.7
>
>
>From 7085d5362b131726bdd8fa3e5bf30217849046e7 Mon Sep 17 00:00:00 2001
>From: Nils Philippsen <nils@redhat.com>
>Date: Tue, 16 Oct 2012 16:57:37 +0200
>Subject: [PATCH 3/3] ppm-load: bring comment in line with reality
>
>---
> operations/external/ppm-load.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/operations/external/ppm-load.c b/operations/external/ppm-load.c
>index 465096d..e22521c 100644
>--- a/operations/external/ppm-load.c
>+++ b/operations/external/ppm-load.c
>@@ -62,7 +62,7 @@ ppm_load_read_header(FILE       *fp,
>     gchar  header[MAX_CHARS_IN_ROW];
>     gint   maxval;
> 
>-    /* Check the PPM file Type P2 or P5 */
>+    /* Check the PPM file Type P3 or P6 */
>     fgets (header,MAX_CHARS_IN_ROW,fp);
> 
>     if (header[0] != ASCII_P ||
>-- 
>1.7.11.7
>

Actions: View | Diff

Attachments on bug 856300: 614202 | 614203 | 633906