From 19f9685dbff7d1f929c61cf99188df917a18811d Mon Sep 17 00:00:00 2001
From: Benjamin Otte <otte@redhat.com>
Date: Sat, 19 Sep 2015 21:24:34 +0200
Subject: [PATCH 1/3] pixops: Fail make_weights functions on OOM

The weights could grow very large under certain circumstances, in
particular in security-relevant conditions, including the testsuite.
By allowing the weight allocation to fail, this can be worked around.

https://bugzilla.gnome.org/show_bug.cgi?id=754387
---
 gdk-pixbuf/pixops/pixops.c | 75 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 55 insertions(+), 20 deletions(-)

diff --git a/gdk-pixbuf/pixops/pixops.c b/gdk-pixbuf/pixops/pixops.c
index e41b286..4cdb5df 100644
--- a/gdk-pixbuf/pixops/pixops.c
+++ b/gdk-pixbuf/pixops/pixops.c
@@ -1478,15 +1478,19 @@ pixops_process (guchar         *dest_buf,
 /* Compute weights for reconstruction by replication followed by
  * sampling with a box filter
  */
-static void
+static gboolean
 tile_make_weights (PixopsFilterDimension *dim,
 		   double                 scale)
 {
   int n = ceil (1 / scale + 1);
-  double *pixel_weights = g_malloc_n (sizeof (double) * SUBSAMPLE, n);
+  double *pixel_weights;
   int offset;
   int i;
 
+  pixel_weights = g_try_malloc_n (sizeof (double) * SUBSAMPLE, n);
+  if (pixel_weights == NULL)
+    return FALSE;
+
   dim->n = n;
   dim->offset = 0;
   dim->weights = pixel_weights;
@@ -1514,13 +1518,15 @@ tile_make_weights (PixopsFilterDimension *dim,
             }
        }
     }
+
+  return TRUE;
 }
 
 /* Compute weights for a filter that, for minification
  * is the same as 'tiles', and for magnification, is bilinear
  * reconstruction followed by a sampling with a delta function.
  */
-static void
+static gboolean
 bilinear_magnify_make_weights (PixopsFilterDimension *dim,
 			       double                 scale)
 {
@@ -1541,7 +1547,9 @@ bilinear_magnify_make_weights (PixopsFilterDimension *dim,
     }
 
   dim->n = n;
-  dim->weights = g_malloc_n (sizeof (double) * SUBSAMPLE, n);
+  dim->weights = g_try_malloc_n (sizeof (double) * SUBSAMPLE, n);
+  if (dim->weights == NULL)
+    return FALSE;
 
   pixel_weights = dim->weights;
 
@@ -1581,6 +1589,8 @@ bilinear_magnify_make_weights (PixopsFilterDimension *dim,
             }
         }
     }
+
+  return TRUE;
 }
 
 /* Computes the integral from b0 to b1 of
@@ -1627,15 +1637,19 @@ linear_box_half (double b0, double b1)
 /* Compute weights for reconstructing with bilinear
  * interpolation, then sampling with a box filter
  */
-static void
+static gboolean
 bilinear_box_make_weights (PixopsFilterDimension *dim,
 			   double                 scale)
 {
   int n = ceil (1/scale + 3.0);
-  double *pixel_weights = g_malloc_n (sizeof (double) * SUBSAMPLE, n);
+  double *pixel_weights;
   double w;
   int offset, i;
 
+  pixel_weights = g_malloc_n (sizeof (double) * SUBSAMPLE, n);
+  if (pixel_weights == NULL)
+    return FALSE;
+
   dim->offset = -1.0;
   dim->n = n;
   dim->weights = pixel_weights;
@@ -1653,9 +1667,11 @@ bilinear_box_make_weights (PixopsFilterDimension *dim,
           *(pixel_weights++) = w * scale;
         }
     }
+
+  return TRUE;
 }
 
-static void
+static gboolean
 make_weights (PixopsFilter     *filter,
 	      PixopsInterpType  interp_type,	      
 	      double            scale_x,
@@ -1664,23 +1680,39 @@ make_weights (PixopsFilter     *filter,
   switch (interp_type)
     {
     case PIXOPS_INTERP_NEAREST:
+    default:
       g_assert_not_reached ();
-      break;
+      return FALSE;
 
     case PIXOPS_INTERP_TILES:
-      tile_make_weights (&filter->x, scale_x);
-      tile_make_weights (&filter->y, scale_y);
-      break;
+      if (!tile_make_weights (&filter->x, scale_x))
+        return FALSE;
+      if (!tile_make_weights (&filter->y, scale_y))
+        {
+          g_free (filter->x.weights);
+          return FALSE;
+        }
+      return TRUE;
       
     case PIXOPS_INTERP_BILINEAR:
-      bilinear_magnify_make_weights (&filter->x, scale_x);
-      bilinear_magnify_make_weights (&filter->y, scale_y);
-      break;
+      if (!bilinear_magnify_make_weights (&filter->x, scale_x))
+        return FALSE;
+      if (!bilinear_magnify_make_weights (&filter->y, scale_y))
+        {
+          g_free (filter->x.weights);
+          return FALSE;
+        }
+      return TRUE;
       
     case PIXOPS_INTERP_HYPER:
-      bilinear_box_make_weights (&filter->x, scale_x);
-      bilinear_box_make_weights (&filter->y, scale_y);
-      break;
+      if (!bilinear_box_make_weights (&filter->x, scale_x))
+        return FALSE;
+      if (!bilinear_box_make_weights (&filter->y, scale_y))
+        {
+          g_free (filter->x.weights);
+          return FALSE;
+        }
+      return TRUE;
     }
 }
 
@@ -1735,7 +1767,8 @@ _pixops_composite_color_real (guchar          *dest_buf,
     }
   
   filter.overall_alpha = overall_alpha / 255.;
-  make_weights (&filter, interp_type, scale_x, scale_y);
+  if (!make_weights (&filter, interp_type, scale_x, scale_y))
+    return;
 
 #ifdef USE_MMX
   if (filter.x.n == 2 && filter.y.n == 2 &&
@@ -1890,7 +1923,8 @@ _pixops_composite_real (guchar          *dest_buf,
     }
   
   filter.overall_alpha = overall_alpha / 255.;
-  make_weights (&filter, interp_type, scale_x, scale_y);
+  if (!make_weights (&filter, interp_type, scale_x, scale_y))
+    return;
 
   if (filter.x.n == 2 && filter.y.n == 2 && dest_channels == 4 &&
       src_channels == 4 && src_has_alpha && !dest_has_alpha)
@@ -2297,7 +2331,8 @@ _pixops_scale_real (guchar        *dest_buf,
     }
   
   filter.overall_alpha = 1.0;
-  make_weights (&filter, interp_type, scale_x, scale_y);
+  if (!make_weights (&filter, interp_type, scale_x, scale_y))
+    return;
 
   if (filter.x.n == 2 && filter.y.n == 2 && dest_channels == 3 && src_channels == 3)
     {
-- 
2.1.4

From edf6fb8d856574bc3bb3a703037f56533229267c Mon Sep 17 00:00:00 2001
From: Benjamin Otte <otte@redhat.com>
Date: Sun, 20 Sep 2015 00:22:42 +0200
Subject: [PATCH 2/3] tga: Wrap TGAColormap struct in its own API

Instead of poking into it directly.
---
 gdk-pixbuf/io-tga.c | 124 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 82 insertions(+), 42 deletions(-)

diff --git a/gdk-pixbuf/io-tga.c b/gdk-pixbuf/io-tga.c
index e382459..70d1892 100644
--- a/gdk-pixbuf/io-tga.c
+++ b/gdk-pixbuf/io-tga.c
@@ -63,8 +63,8 @@ typedef struct _IOBuffer IOBuffer;
 typedef struct _TGAHeader TGAHeader;
 typedef struct _TGAFooter TGAFooter;
 
-typedef struct _TGAColormap TGAColormap;
 typedef struct _TGAColor TGAColor;
+typedef struct _TGAColormap TGAColormap;
 
 typedef struct _TGAContext TGAContext;
 
@@ -101,15 +101,15 @@ struct _TGAFooter {
 	} sig;
 };
 
-struct _TGAColormap {
-	gint size;
-	TGAColor *cols;
-};
-
 struct _TGAColor {
 	guchar r, g, b, a;
 };
 
+struct _TGAColormap {
+	guint n_colors;
+	TGAColor colors[1];
+};
+
 struct _TGAContext {
 	TGAHeader *hdr;
 	guint rowstride;
@@ -234,6 +234,51 @@ static void free_buffer(guchar *pixels, gpointer data)
 	g_free(pixels);
 }
 
+static TGAColormap *
+colormap_new (guint n_colors)
+{
+  TGAColormap *cmap;
+
+  g_assert (n_colors <= G_MAXUINT16);
+
+  cmap = g_try_malloc0 (sizeof (TGAColormap) + (MAX (n_colors, 1) - 1) * sizeof (TGAColor));
+  if (cmap == NULL)
+    return NULL;
+
+  cmap->n_colors = n_colors;
+
+  return cmap;
+}
+
+static const TGAColor *
+colormap_get_color (TGAColormap *cmap,
+                    guint        id)
+{
+  static const TGAColor transparent_black = { 0, 0, 0, 0 };
+
+  if (id >= cmap->n_colors)
+    return &transparent_black;
+
+  return &cmap->colors[id];
+}
+
+static void
+colormap_set_color (TGAColormap    *cmap,
+                    guint           id,
+                    const TGAColor *color)
+{
+  if (id >= cmap->n_colors)
+    return;
+
+  cmap->colors[id] = *color;
+}
+
+static void
+colormap_free (TGAColormap *cmap)
+{
+  g_free (cmap);
+}
+
 static GdkPixbuf *get_contiguous_pixbuf (guint width, 
 					 guint height, 
 					 gboolean has_alpha)
@@ -364,11 +409,12 @@ static void parse_data_for_row_pseudocolor(TGAContext *ctx)
 	guchar *p = ctx->pptr;
 
 	for (; upper_bound; upper_bound--, s++) {
-		*p++ = ctx->cmap->cols[*s].r;
-		*p++ = ctx->cmap->cols[*s].g;
-		*p++ = ctx->cmap->cols[*s].b;
+                const TGAColor *color = colormap_get_color (ctx->cmap, *s);
+		*p++ = color->r;
+		*p++ = color->g;
+		*p++ = color->b;
 		if (ctx->hdr->cmap_bpp == 32)
-			*p++ = ctx->cmap->cols[*s].a;
+			*p++ = color->a;
 	}
 }
 
@@ -435,7 +481,7 @@ static gboolean parse_data_for_row(TGAContext *ctx, GError **err)
 	return TRUE;
 }
 
-static void write_rle_data(TGAContext *ctx, TGAColor *color, guint *rle_count)
+static void write_rle_data(TGAContext *ctx, const TGAColor *color, guint *rle_count)
 {
 	for (; *rle_count; (*rle_count)--) {
 		g_memmove(ctx->pptr, (guchar *) color, ctx->pbuf->n_channels);
@@ -463,7 +509,7 @@ static guint parse_rle_data_pseudocolor(TGAContext *ctx)
 				return --n;
 			} else {
 				rle_num = (tag & 0x7f) + 1;
-				write_rle_data(ctx, &ctx->cmap->cols[*s], &rle_num);
+				write_rle_data(ctx, colormap_get_color (ctx->cmap, *s), &rle_num);
 				s++, n++;
 				if (ctx->pbuf_bytes_done == ctx->pbuf_bytes) {
 					ctx->done = TRUE;
@@ -476,14 +522,12 @@ static guint parse_rle_data_pseudocolor(TGAContext *ctx)
 				return --n;
 			} else {
 				for (; raw_num; raw_num--) {
-					*ctx->pptr++ =
-						ctx->cmap->cols[*s].r;
-					*ctx->pptr++ =
-						ctx->cmap->cols[*s].g;
-					*ctx->pptr++ =
-						ctx->cmap->cols[*s].b;
+                                        const TGAColor *color = colormap_get_color (ctx->cmap, *s);
+					*ctx->pptr++ = color->r;
+					*ctx->pptr++ = color->g;
+					*ctx->pptr++ = color->b;
 					if (ctx->pbuf->n_channels == 4)
-						*ctx->pptr++ = ctx->cmap->cols[*s].a;
+						*ctx->pptr++ = color->a;
 					s++, n++;
 					ctx->pbuf_bytes_done += ctx->pbuf->n_channels;
 					if (ctx->pbuf_bytes_done == ctx->pbuf_bytes) {
@@ -667,8 +711,9 @@ static gboolean parse_rle_data(TGAContext *ctx, GError **err)
 
 static gboolean try_colormap(TGAContext *ctx, GError **err)
 {
-	static guchar *p;
-	static guint n;
+        TGAColor color;
+	guchar *p;
+	guint i, n_colors;
 
 	g_return_val_if_fail(ctx != NULL, FALSE);
 
@@ -679,41 +724,38 @@ static gboolean try_colormap(TGAContext *ctx, GError **err)
 			return FALSE;
         }
 
-	ctx->cmap = g_try_malloc(sizeof(TGAColormap));
+        n_colors = LE16(ctx->hdr->cmap_n_colors);
+        ctx->cmap = colormap_new (n_colors);
 	if (!ctx->cmap) {
 		g_set_error_literal(err, GDK_PIXBUF_ERROR, GDK_PIXBUF_ERROR_INSUFFICIENT_MEMORY,
-                                    _("Cannot allocate colormap structure"));
-		return FALSE;
-	}
-	ctx->cmap->size = LE16(ctx->hdr->cmap_n_colors);
-	ctx->cmap->cols = g_try_malloc(sizeof(TGAColor) * ctx->cmap->size);
-	if (!ctx->cmap->cols) {
-		g_set_error_literal(err, GDK_PIXBUF_ERROR, GDK_PIXBUF_ERROR_INSUFFICIENT_MEMORY,
-                                    _("Cannot allocate colormap entries"));
+                                    _("Cannot allocate colormap"));
 		return FALSE;
 	}
 
 	p = ctx->in->data;
-	for (n = 0; n < ctx->cmap->size; n++) {
+        color.a = 255;
+
+	for (i = 0; i < n_colors; i++) {
 		if ((ctx->hdr->cmap_bpp == 15) || (ctx->hdr->cmap_bpp == 16)) {
 			guint16 col = p[0] + (p[1] << 8);
-			ctx->cmap->cols[n].b = (col >> 7) & 0xf8;
-			ctx->cmap->cols[n].g = (col >> 2) & 0xf8;
-			ctx->cmap->cols[n].r = col << 3;
+			color.b = (col >> 7) & 0xf8;
+			color.g = (col >> 2) & 0xf8;
+			color.r = col << 3;
 			p += 2;
 		}
 		else if ((ctx->hdr->cmap_bpp == 24) || (ctx->hdr->cmap_bpp == 32)) {
-			ctx->cmap->cols[n].b = *p++;
-			ctx->cmap->cols[n].g = *p++;
-			ctx->cmap->cols[n].r = *p++;
+			color.b = *p++;
+			color.g = *p++;
+			color.r = *p++;
 			if (ctx->hdr->cmap_bpp == 32)
-				ctx->cmap->cols[n].a = *p++;
+				color.a = *p++;
 		} else {
 			g_set_error_literal(err, GDK_PIXBUF_ERROR, 
                                             GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
                                             _("Unexpected bitdepth for colormap entries"));
 			return FALSE;
 		}
+                colormap_set_color (ctx->cmap, i, &color);
 	}
 	ctx->in = io_buffer_free_segment(ctx->in, ctx->cmap_size, err);
 	if (!ctx->in)
@@ -944,10 +986,8 @@ static gboolean gdk_pixbuf__tga_stop_load(gpointer data, GError **err)
 			       	       ctx->udata);
 	}
 	g_free (ctx->hdr);
-	if (ctx->cmap) {
-	  g_free (ctx->cmap->cols);
-	  g_free (ctx->cmap);
-	}
+	if (ctx->cmap)
+          colormap_free (ctx->cmap);
 	if (ctx->pbuf)
 		g_object_unref (ctx->pbuf);
 	if (ctx->in && ctx->in->size)
-- 
2.1.4

From 6ddca835100107e6b5841ce9d56074f6d98c387e Mon Sep 17 00:00:00 2001
From: Benjamin Otte <otte@redhat.com>
Date: Sun, 20 Sep 2015 00:29:59 +0200
Subject: [PATCH 3/3] io-tga: Colormaps are always present, so always parse
 them.

We might end up with a colormap with 0 entries, but whatever, it's a
colormap.
---
 gdk-pixbuf/io-tga.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/gdk-pixbuf/io-tga.c b/gdk-pixbuf/io-tga.c
index 70d1892..156a416 100644
--- a/gdk-pixbuf/io-tga.c
+++ b/gdk-pixbuf/io-tga.c
@@ -356,9 +356,8 @@ static gboolean fill_in_context(TGAContext *ctx, GError **err)
 		 || (ctx->hdr->type == TGA_TYPE_RLE_TRUECOLOR)
 		 || (ctx->hdr->type == TGA_TYPE_RLE_GRAYSCALE));
 
-	if (ctx->hdr->has_cmap)
-		ctx->cmap_size = ((ctx->hdr->cmap_bpp + 7) >> 3) *
-			LE16(ctx->hdr->cmap_n_colors);
+        ctx->cmap_size = ((ctx->hdr->cmap_bpp + 7) >> 3) *
+                LE16(ctx->hdr->cmap_n_colors);
 
 	alpha = ((ctx->hdr->bpp == 16) || 
 		 (ctx->hdr->bpp == 32) ||
@@ -717,13 +716,6 @@ static gboolean try_colormap(TGAContext *ctx, GError **err)
 
 	g_return_val_if_fail(ctx != NULL, FALSE);
 
-        if (ctx->cmap_size == 0) {
-		g_set_error_literal(err, GDK_PIXBUF_ERROR,
-                                    GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
-                                    _("Image is corrupted or truncated"));
-			return FALSE;
-        }
-
         n_colors = LE16(ctx->hdr->cmap_n_colors);
         ctx->cmap = colormap_new (n_colors);
 	if (!ctx->cmap) {
@@ -865,7 +857,7 @@ static gboolean try_preload(TGAContext *ctx, GError **err)
 			return TRUE;
 		}
 	}
-	if (ctx->hdr->has_cmap && !ctx->cmap) {
+	if (!ctx->cmap) {
 		if (ctx->in->size >= ctx->cmap_size) {
 			if (!try_colormap(ctx, err))
 				return FALSE;
-- 
2.1.4

