[gimp/gimp-2-8] app: limit allowable tile data size in XCFs



commit d7a9e6079d3e65baa516f302eb285fb2bbd97c2f
Author: Ell <ell_se yahoo com>
Date:   Sun Jul 9 17:37:43 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-compressed tiles, a buffer large enough to hold the
    entire on-disk tile data, up to 2GB, is allocated on the heap.  This
    allows a malicious XCF, which may be much smaller than 2GB, to cause
    a very large allocation, which may abort the program if there's not
    enough memory (or not enough address space, which is much more likely
    on 32-bit systems.)

 app/xcf/xcf-load.c    |   39 +++++++++++++++++++++++++++------------
 app/xcf/xcf-private.h |   11 ++++++++++-
 app/xcf/xcf-save.c    |   21 +++++++++++++++++++--
 3 files changed, 56 insertions(+), 15 deletions(-)
---
diff --git a/app/xcf/xcf-load.c b/app/xcf/xcf-load.c
index 67cc6d4..83f93fb 100644
--- a/app/xcf/xcf-load.c
+++ b/app/xcf/xcf-load.c
@@ -1445,15 +1445,16 @@ static gboolean
 xcf_load_level (XcfInfo     *info,
                 TileManager *tiles)
 {
-  guint32 saved_pos;
-  guint32 offset, offset2;
-  guint ntiles;
-  gint  width;
-  gint  height;
-  gint  i;
-  gint  fail;
-  Tile *previous;
-  Tile *tile;
+  guint32  saved_pos;
+  guint32  offset, offset2;
+  guint32  max_data_length;
+  guint    ntiles;
+  gint     width;
+  gint     height;
+  gint     i;
+  gint     fail;
+  Tile    *previous;
+  Tile    *tile;
 
   info->cp += xcf_read_int32 (info->fp, (guint32 *) &width, 1);
   info->cp += xcf_read_int32 (info->fp, (guint32 *) &height, 1);
@@ -1462,6 +1463,13 @@ xcf_load_level (XcfInfo     *info,
       height != tile_manager_height (tiles))
     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 = TILE_WIDTH * TILE_HEIGHT * 4 *
+                    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.
@@ -1499,14 +1507,21 @@ xcf_load_level (XcfInfo     *info,
       /* if the offset is 0 then we need to read in the maximum possible
          allowing for negative compression */
       if (offset2 == 0)
-        offset2 = offset + TILE_WIDTH * TILE_WIDTH * 4 * 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: %u",
+                        offset2 - offset);
+          return FALSE;
+        }
+
       /* get the tile from the tile manager */
       tile = tile_manager_get (tiles, i, TRUE, TRUE);
 
diff --git a/app/xcf/xcf-private.h b/app/xcf/xcf-private.h
index ddca589..db328ec 100644
--- a/app/xcf/xcf-private.h
+++ b/app/xcf/xcf-private.h
@@ -22,11 +22,20 @@
  * SECTION:xcf-private
  * @Short_description:Common definitions for the XCF functions
  *
- * Common enum and struct definitions for the XCF functions
+ * Common macro, enum and struct definitions for the XCF functions
  */
 
 
 /**
+ * XCF_TILE_MAX_DATA_LENGTH_FACTOR:
+ *
+ * The ratio between the maximal allowable on-disk tile data size, and the
+ * uncompressed tile data size.
+ */
+#define XCF_TILE_MAX_DATA_LENGTH_FACTOR 1.5
+
+
+/**
 * PropType:
 * @PROP_END:                 marks the end of any property list
 * @PROP_COLORMAP:            stores the color map in indexed images
diff --git a/app/xcf/xcf-save.c b/app/xcf/xcf-save.c
index 0e713ed..f75412b 100644
--- a/app/xcf/xcf-save.c
+++ b/app/xcf/xcf-save.c
@@ -1388,6 +1388,7 @@ xcf_save_level (XcfInfo      *info,
   guint32 *next_offset;
   guint32  saved_pos;
   guint32  offset;
+  guint32  max_data_length;
   guint32  width;
   guint32  height;
   guint    ntiles;
@@ -1402,10 +1403,16 @@ xcf_save_level (XcfInfo      *info,
   xcf_write_int32_check_error (info, (guint32 *) &width, 1);
   xcf_write_int32_check_error (info, (guint32 *) &height, 1);
 
+  /* 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 = TILE_WIDTH * TILE_HEIGHT * tile_manager_bpp (level) *
+                    XCF_TILE_MAX_DATA_LENGTH_FACTOR /* = 1.5, currently */;
+
   /* allocate a temporary buffer to store the rle data before it is
      written to disk */
-  rlebuf =
-    g_malloc (TILE_WIDTH * TILE_HEIGHT * tile_manager_bpp (level) * 1.5);
+  rlebuf = g_malloc (max_data_length);
 
   ntiles = level->ntile_rows * level->ntile_cols;
 
@@ -1451,6 +1458,16 @@ xcf_save_level (XcfInfo      *info,
               break;
             }
 
+          /* 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_error ("xcf: invalid tile data length: %u",
+                       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]