[gdk-pixbuf] io-xbm: Fix potential overflows in read_bitmap_file_data()



commit 3dacbb48f0ca67d1b7124f6de58796a517dbed53
Author: Philip Withnall <withnall endlessm com>
Date:   Tue Jan 17 09:39:06 2017 +0000

    io-xbm: Fix potential overflows in read_bitmap_file_data()
    
    ww and hh are both potentially tainted data (as they come from a
    potentially attacker controlled file), so we need to ensure that all
    arithmetic is bounds checked.
    
    Coverity ID: 1388540
    
    https://bugzilla.gnome.org/show_bug.cgi?id=777374

 gdk-pixbuf/io-xbm.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)
---
diff --git a/gdk-pixbuf/io-xbm.c b/gdk-pixbuf/io-xbm.c
index 95dbfe8..d21ee1f 100644
--- a/gdk-pixbuf/io-xbm.c
+++ b/gdk-pixbuf/io-xbm.c
@@ -151,7 +151,7 @@ read_bitmap_file_data (FILE    *fstream,
 {
        guchar *bits = NULL;            /* working variable */
        char line[MAX_SIZE];            /* input line from file */
-       int size;                       /* number of bytes of data */
+       guint size;                     /* number of bytes of data */
        char name_and_type[MAX_SIZE];   /* an input line */
        char *type;                     /* for parsing */
        int value;                      /* from an input line */
@@ -227,21 +227,37 @@ read_bitmap_file_data (FILE    *fstream,
                if (!ww || !hh)
                        RETURN (FALSE);
 
+               /* Choose @padding so @size is even if @version10p is %TRUE.
+                * If @version10p is %FALSE, @size could be even or odd. */
                if ((ww % 16) && ((ww % 16) < 9) && version10p)
                        padding = 1;
                else
                        padding = 0;
 
+               /* Check for overflow for the bytes_per_line calculation. */
+               if (ww > G_MAXUINT - 7)
+                       RETURN (FALSE);
+
                bytes_per_line = (ww+7)/8 + padding;
+               g_assert (!version10p || (bytes_per_line % 2) == 0);
+
+               /* size = bytes_per_line * hh */
+               if (!g_uint_checked_mul (&size, bytes_per_line, hh))
+                       RETURN (FALSE);
 
-               size = bytes_per_line * hh;
-                if (size / bytes_per_line != hh) /* overflow */
-                        RETURN (FALSE);
                bits = g_malloc (size);
 
                if (version10p) {
                        unsigned char *ptr;
-                       int bytes;
+                       guint bytes;
+
+                       /* @bytes is guaranteed not to overflow (which could
+                        * happen if @size is the odd-valued %G_MAXUINT: @bytes would reach
+                        * %G_MAXUINT-1 in the loop, then be incremented to %G_MAXUINT+1 on the
+                        * next iteration) because @bytes_per_line is guaranteed to be even if
+                        * @version10p is %TRUE (due to the selection of
+                        * @padding in that case), so @size must be even too. */
+                       g_assert ((size % 2) == 0);
 
                        for (bytes = 0, ptr = bits; bytes < size; (bytes += 2)) {
                                if ((value = next_int (fstream)) < 0)
@@ -252,7 +268,7 @@ read_bitmap_file_data (FILE    *fstream,
                        }
                } else {
                        unsigned char *ptr;
-                       int bytes;
+                       guint bytes;
 
                        for (bytes = 0, ptr = bits; bytes < size; bytes++, ptr++) {
                                if ((value = next_int (fstream)) < 0) 


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