[gdk-pixbuf] ico: Be more careful when parsing headers



commit 88af50a864195da1a4f7bda5f02539704fbda599
Author: Matthias Clasen <mclasen redhat com>
Date:   Wed Aug 3 12:40:48 2016 -0400

    ico: Be more careful when parsing headers
    
    There is some redundancy between the ico directory and the
    bitmap image header. If the two disagree on the icon dimensions,
    just toss the image, instead of risking crashes or OOM later. Also
    add some more debug spew that helped in tracking this down, and
    make error messages more unique.
    
    The commit also includes a test image that has an example of
    this discrepancy and triggers the early exit.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=769170

 gdk-pixbuf/io-ico.c |  136 +++++++++++++++++++++++++++++----------------------
 1 files changed, 78 insertions(+), 58 deletions(-)
---
diff --git a/gdk-pixbuf/io-ico.c b/gdk-pixbuf/io-ico.c
index 797f6d1..e1f13c1 100644
--- a/gdk-pixbuf/io-ico.c
+++ b/gdk-pixbuf/io-ico.c
@@ -23,6 +23,8 @@
  */
 
 #undef DUMPBIH
+#define DEBUG(s)
+
 /*
 
 Icons are just like BMP's, except for the header.
@@ -75,14 +77,14 @@ struct BitmapInfoHeader {
 };
 
 #ifdef DUMPBIH
-/* 
+/*
 
 DumpBIH printf's the values in a BitmapInfoHeader to the screen, for 
 debugging purposes.
 
 */
 static void DumpBIH(unsigned char *BIH)
-{                              
+{
        printf("biSize      = %i \n",
               (int)(BIH[3] << 24) + (BIH[2] << 16) + (BIH[1] << 8) + (BIH[0]));
        printf("biWidth     = %i \n",
@@ -125,6 +127,8 @@ struct headerpair {
 /* Score the various parts of the icon */
 struct ico_direntry_data {
        gint ImageScore;
+        gint width;
+        gint height;
        gint DIBoffset;
        gint x_hot;
        gint y_hot;
@@ -220,10 +224,10 @@ static void DecodeHeader(guchar *Data, gint Bytes,
 
        /* First word should be 0 according to specs */
        if (((Data[1] << 8) + Data[0]) != 0) {
-               g_set_error_literal (error,
-                                    GDK_PIXBUF_ERROR,
-                                    GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
-                                    _("Invalid header in icon"));
+               g_set_error (error,
+                            GDK_PIXBUF_ERROR,
+                            GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
+                            _("Invalid header in icon (%s)"), "first word");
                return;
 
        }
@@ -234,18 +238,19 @@ static void DecodeHeader(guchar *Data, gint Bytes,
 
        /* If it is not a cursor make sure it is actually an icon */
        if (!State->cursor && imgtype != 1) {
-               g_set_error_literal (error,
-                                    GDK_PIXBUF_ERROR,
-                                    GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
-                                    _("Invalid header in icon"));
+               g_set_error (error,
+                            GDK_PIXBUF_ERROR,
+                            GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
+                            _("Invalid header in icon (%s)"), "image type");
                return;
        }
 
-
        IconCount = (Data[5] << 8) + (Data[4]);
        
        State->HeaderSize = 6 + IconCount*16;
 
+        DEBUG(g_print ("Image type: %d (%s)\nImage count: %d\n", imgtype, imgtype == 2 ? "cursor" : "icon", 
IconCount));
+
        if (State->HeaderSize>State->BytesInHeaderBuf) {
                guchar *tmp=g_try_realloc(State->HeaderBuf,State->HeaderSize);
                if (!tmp) {
@@ -259,10 +264,6 @@ static void DecodeHeader(guchar *Data, gint Bytes,
                State->BytesInHeaderBuf = State->HeaderSize;
        }
        if (Bytes < State->HeaderSize) {
-               g_set_error_literal (error,
-                                     GDK_PIXBUF_ERROR,
-                                     GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
-                                     _("Not enough bytes for header"));
                return;
        }
 
@@ -272,17 +273,37 @@ static void DecodeHeader(guchar *Data, gint Bytes,
        State->entries = 0;
        Ptr = Data + 6;
        for (I=0;I<IconCount;I++) {
+                int width;
+                int height;
+                int x_hot;
+                int y_hot;
+                int data_size;
+                int data_offset;
+
+                width = Ptr[0];
+                height = Ptr[1];
+               x_hot = (Ptr[5] << 8) + Ptr[4];
+               y_hot = (Ptr[7] << 8) + Ptr[6];
+                data_size = (Ptr[11] << 24) + (Ptr[10] << 16) + (Ptr[9] << 8) + (Ptr[8]);
+               data_offset = (Ptr[15] << 24) + (Ptr[14] << 16) + (Ptr[13] << 8) + (Ptr[12]);
+                DEBUG(g_print ("Image %d: %d x %d\n\tPalette: %d\n", I, width, height, {tr[2]);
+                if (imgtype == 2)
+                  g_print ("\tHotspot: %d x %d\n", x_hot, y_hot);
+                else
+                  g_print ("\tColor planes: %d\n\tBits per pixel: %d\n", x_hot, y_hot);
+                g_print ("\tSize: %d\n\tOffset: %d\n", data_size, data_offset);)
+
                entry = g_new0 (struct ico_direntry_data, 1);
-               entry->ImageScore = (Ptr[11] << 24) + (Ptr[10] << 16) + (Ptr[9] << 8) + (Ptr[8]);
-               if (entry->ImageScore == 0)
-                       entry->ImageScore = 256;
-               entry->x_hot = (Ptr[5] << 8) + Ptr[4];
-               entry->y_hot = (Ptr[7] << 8) + Ptr[6];
-               entry->DIBoffset = (Ptr[15]<<24)+(Ptr[14]<<16)+
-                                  (Ptr[13]<<8) + (Ptr[12]);
+               entry->ImageScore = data_size;
+
+                entry->width = width ? width : 256;
+                entry->height = height ? height : 256;
+               entry->x_hot = x_hot;
+               entry->y_hot = y_hot;
+               entry->DIBoffset = data_offset;
                State->entries = g_list_insert_sorted (State->entries, entry, compare_direntry_scores);
                Ptr += 16;
-       } 
+       }
 
        /* Now go through and find one we can parse */
        entry = NULL;
@@ -290,10 +311,10 @@ static void DecodeHeader(guchar *Data, gint Bytes,
                entry = l->data;
 
                if (entry->DIBoffset < 0) {
-                       g_set_error_literal (error,
-                                            GDK_PIXBUF_ERROR,
-                                            GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
-                                            _("Invalid header in icon"));
+                       g_set_error (error,
+                                    GDK_PIXBUF_ERROR,
+                                    GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
+                                    _("Invalid header in icon (%s)"), "dib offset");
                        return;
                }
 
@@ -301,10 +322,10 @@ static void DecodeHeader(guchar *Data, gint Bytes,
                State->HeaderSize = entry->DIBoffset + 40; /* 40 = sizeof(InfoHeader) */
 
                if (State->HeaderSize < 0) {
-                       g_set_error_literal (error,
-                                            GDK_PIXBUF_ERROR,
-                                            GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
-                                            _("Invalid header in icon"));
+                       g_set_error (error,
+                                    GDK_PIXBUF_ERROR,
+                                    GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
+                                    _("Invalid header in icon (%s)"), "header size");
                        return;
                }
 
@@ -351,9 +372,9 @@ static void DecodeHeader(guchar *Data, gint Bytes,
 
 #ifdef DUMPBIH
        DumpBIH(BIH);
-#endif 
+#endif
        /* Add the palette to the headersize */
-               
+
        State->Header.width =
            (int)(BIH[7] << 24) + (BIH[6] << 16) + (BIH[5] << 8) + (BIH[4]);
        if (State->Header.width == 0)
@@ -364,12 +385,28 @@ static void DecodeHeader(guchar *Data, gint Bytes,
            /* /2 because the BIH height includes the transparency mask */
        if (State->Header.height == 0)
                State->Header.height = 256;
+
+       /* Negative heights mean top-down pixel-order */
+       if (State->Header.height < 0) {
+               State->Header.height = -State->Header.height;
+               State->Header.Negative = 1;
+       }
+       if (State->Header.width < 0) {
+               State->Header.width = -State->Header.width;
+       }
+
+        if (State->Header.width != entry->width ||
+            State->Header.height != entry->height) {
+               g_set_error (error,
+                             GDK_PIXBUF_ERROR,
+                             GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
+                             _("Invalid header in icon (%s)"), "image size");
+               return;
+        }
+
        State->Header.depth = (BIH[15] << 8) + (BIH[14]);
+       State->Type = State->Header.depth;
 
-       State->Type = State->Header.depth;      
-       if (State->Lines>=State->Header.height)
-               State->Type = 1; /* The transparency mask is 1 bpp */
-       
        /* Determine the  palette size. If the header indicates 0, it
           is actually the maximum for the bpp. You have to love the
           guys who made the spec. */
@@ -385,10 +422,10 @@ static void DecodeHeader(guchar *Data, gint Bytes,
        State->HeaderSize+=I;
        
        if (State->HeaderSize < 0) {
-               g_set_error_literal (error,
-                                     GDK_PIXBUF_ERROR,
-                                     GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
-                                     _("Invalid header in icon"));
+               g_set_error (error,
+                             GDK_PIXBUF_ERROR,
+                             GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
+                             _("Invalid header in icon (%s)"), "palette size");
                return;
        }
 
@@ -405,24 +442,9 @@ static void DecodeHeader(guchar *Data, gint Bytes,
                State->BytesInHeaderBuf = State->HeaderSize;
        }
        if (Bytes < State->HeaderSize) {
-               g_set_error_literal (error,
-                                     GDK_PIXBUF_ERROR,
-                                     GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
-                                     _("Not enough bytes for header"));
                return;
        }
 
-       /* Negative heights mean top-down pixel-order */
-       if (State->Header.height < 0) {
-               State->Header.height = -State->Header.height;
-               State->Header.Negative = 1;
-       }
-       if (State->Header.width < 0) {
-               State->Header.width = -State->Header.width;
-       }
-       g_assert (State->Header.width > 0);
-       g_assert (State->Header.height > 0);
-
         if (State->Type == 32)
                 State->LineWidth = State->Header.width * 4;
         else if (State->Type == 24)
@@ -465,7 +487,6 @@ static void DecodeHeader(guchar *Data, gint Bytes,
 
 
        if (State->pixbuf == NULL) {
-#if 1
                if (State->size_func) {
                        gint width = State->Header.width;
                        gint height = State->Header.height;
@@ -476,7 +497,6 @@ static void DecodeHeader(guchar *Data, gint Bytes,
                                return;
                        }
                }
-#endif
 
                State->pixbuf =
                    gdk_pixbuf_new(GDK_COLORSPACE_RGB, TRUE, 8,


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