[gimp] app: limit allowable tile data size in XCFs



commit 6b842930783dd4a7b5587ff74788b8d3b7e37ef2
Author: Ell <ell_se yahoo com>
Date:   Sun Jul 9 17:20:30 2017 -0400

    app: limit allowable tile data size in XCFs
    
    When loading tiles from an XCF, reject tiles whose on-disk size is
    greater than 1.5 times the size of an uncompressed tile -- a limit
    that is already present for the last tile in the buffer.  This
    should allow for the possibility of negative compression, while
    restricting placing a realistic limit.
    
    Currently, no limit is placed on the on-disk tile data size.  When
    loading RLE- and zlib-compressed tiles, a buffer large enough to
    hold the entire on-disk tile data, up to 2GB, is allocated on the
    stack, and the data is read into it.  If the file is smaller than
    the reported tile data size, the area of the buffer past the end
    of the file is not touched.  This allows a malicious XCF to write
    up to 2GB of arbitrary data, at an arbitrary offset, up to 2GB,
    below the stack.
    
    Note that a similar issue had existed for earlier versions of GIMP
    (see commit d7a9e6079d3e65baa516f302eb285fb2bbd97c2f), however,
    since prior to 2.9 the tile data buffer was allocated on the heap,
    the potential risk is far smaller.

 app/xcf/xcf-load.c    |   21 ++++++++++++++++++---
 app/xcf/xcf-private.h |    5 +++--
 app/xcf/xcf-save.c    |   20 +++++++++++++++++++-
 3 files changed, 40 insertions(+), 6 deletions(-)
---
diff --git a/app/xcf/xcf-load.c b/app/xcf/xcf-load.c
index 5cc9304..fc0b4e0 100644
--- a/app/xcf/xcf-load.c
+++ b/app/xcf/xcf-load.c
@@ -1899,6 +1899,7 @@ xcf_load_level (XcfInfo    *info,
   goffset     saved_pos;
   goffset     offset;
   goffset     offset2;
+  goffset     max_data_length;
   gint        n_tile_rows;
   gint        n_tile_cols;
   guint       ntiles;
@@ -1917,6 +1918,13 @@ xcf_load_level (XcfInfo    *info,
       height != gegl_buffer_get_height (buffer))
     return FALSE;
 
+  /* maximal allowable size of on-disk tile data.  make it somewhat bigger than
+   * the uncompressed tile size, to allow for the possibility of negative
+   * compression.
+   */
+  max_data_length = XCF_TILE_WIDTH * XCF_TILE_HEIGHT * bpp *
+                    XCF_TILE_MAX_DATA_LENGTH_FACTOR /* = 1.5, currently */;
+
   /* read in the first tile offset.
    *  if it is '0', then this tile level is empty
    *  and we can simply return.
@@ -1957,14 +1965,21 @@ xcf_load_level (XcfInfo    *info,
        * allowing for negative compression
        */
       if (offset2 == 0)
-        offset2 = offset + XCF_TILE_WIDTH * XCF_TILE_WIDTH * bpp * 1.5;
-                                        /* 1.5 is probably more
-                                           than we need to allow */
+        offset2 = offset + max_data_length;
 
       /* seek to the tile offset */
       if (! xcf_seek_pos (info, offset, NULL))
         return FALSE;
 
+      if (offset2 < offset || offset2 - offset > max_data_length)
+        {
+          gimp_message (info->gimp, G_OBJECT (info->progress),
+                        GIMP_MESSAGE_ERROR,
+                        "invalid tile data length: %" G_GOFFSET_FORMAT,
+                        offset2 - offset);
+          return FALSE;
+        }
+
       /* get the tile from the tile manager */
       gimp_gegl_buffer_get_tile_rect (buffer,
                                       XCF_TILE_WIDTH, XCF_TILE_HEIGHT,
diff --git a/app/xcf/xcf-private.h b/app/xcf/xcf-private.h
index aa80876..4b18fc8 100644
--- a/app/xcf/xcf-private.h
+++ b/app/xcf/xcf-private.h
@@ -19,8 +19,9 @@
 #define __XCF_PRIVATE_H__
 
 
-#define XCF_TILE_WIDTH  64
-#define XCF_TILE_HEIGHT 64
+#define XCF_TILE_WIDTH                  64
+#define XCF_TILE_HEIGHT                 64
+#define XCF_TILE_MAX_DATA_LENGTH_FACTOR 1.5
 
 typedef enum
 {
diff --git a/app/xcf/xcf-save.c b/app/xcf/xcf-save.c
index 82c384c..42ced1f 100644
--- a/app/xcf/xcf-save.c
+++ b/app/xcf/xcf-save.c
@@ -1501,6 +1501,7 @@ xcf_save_level (XcfInfo     *info,
   goffset    *next_offset;
   goffset     saved_pos;
   goffset     offset;
+  goffset     max_data_length;
   guint32     width;
   guint32     height;
   gint        bpp;
@@ -1522,11 +1523,18 @@ xcf_save_level (XcfInfo     *info,
 
   saved_pos = info->cp;
 
+  /* maximal allowable size of on-disk tile data.  make it somewhat bigger than
+   * the uncompressed tile size, to allow for the possibility of negative
+   * compression.  xcf_load_level() enforces this limit.
+   */
+  max_data_length = XCF_TILE_WIDTH * XCF_TILE_HEIGHT * bpp *
+                    XCF_TILE_MAX_DATA_LENGTH_FACTOR /* = 1.5, currently */;
+
   /* allocate a temporary buffer to store the rle data before it is
    * written to disk
    */
   if (info->compression == COMPRESS_RLE)
-    rlebuf = g_alloca (XCF_TILE_WIDTH * XCF_TILE_HEIGHT * bpp * 1.5);
+    rlebuf = g_alloca (max_data_length);
 
   n_tile_rows = gimp_gegl_buffer_get_n_tile_rows (buffer, XCF_TILE_HEIGHT);
   n_tile_cols = gimp_gegl_buffer_get_n_tile_cols (buffer, XCF_TILE_WIDTH);
@@ -1581,6 +1589,16 @@ xcf_save_level (XcfInfo     *info,
           return FALSE;
         }
 
+      /* make sure the on-disk tile data didn't end up being too big.
+       * xcf_load_level() would refuse to load the file if it did.
+       */
+      if (info->cp < offset || info->cp - offset > max_data_length)
+        {
+          g_message ("xcf: invalid tile data length: %" G_GOFFSET_FORMAT,
+                     info->cp - offset);
+          return FALSE;
+        }
+
       /* the next tile's offset is after the tile we just wrote */
       offset = info->cp;
     }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]