xorl %eax, %eax

CVE-2009-1194: Pango Integer Overflow

leave a comment »

This library, Pango, is widely used in applications for text rendering and this vulnerability affects releases prior to 1.24 release. It was reported by Will Drewry, founding member of oCERT who works in Google Security Team. As Karl Tomlinson noticed, this bug affects Mozilla Firefox as well since it uses this library. Of course, you can disable it, but I believe most people would have it enabled. Anyway, here is the bug:

4  * Copyright (C) 1999 Red Hat Software
  ...
49 /**
50  * pango_glyph_string_set_size:
51  * @string:    a #PangoGlyphString.
52  * @new_len:   the new length of the string.
53  *
54  * Resize a glyph string to the given length.
55  */
56 void
57 pango_glyph_string_set_size (PangoGlyphString *string, gint new_len)
58 {
59   g_return_if_fail (new_len >= 0);
60
61   while (new_len > string->space)
62     {
63       if (string->space == 0)
64         string->space = 1;
65       else
66         string->space *= 2;
67
68       if (string->space < 0)
69         {
70           g_warning ("glyph string length overflows maximum integer size, truncated");
71           new_len = string->space = G_MAXINT - 8;
72         }
73     }
74
75   string->glyphs = g_realloc (string->glyphs, string->space * sizeof (PangoGlyphInfo));
76   string->log_clusters = g_realloc (string->log_clusters, string->space * sizeof (gint));
77   string->num_glyphs = new_len;
78 }

This function is from 1.23 release of that project and can be found at pango/glyphstring.c. The bug is easy to spot, if the new length (new_len) is larger than the string’s space (line 61), it doubles it (line 66) or sets it to one (line 64) if it was zero, and then, at lines 75 and 76 it uses g_realloc() to allocate space for the new string. These size calculations in the allocations can easily lead to integer overflows. To fix this, the following patch was applied:

       if (string->space == 0)
-       string->space = 1;
+       {
+         string->space = 4;
+       }
       else
-       string->space *= 2;
-
-      if (string->space < 0)
        {
-         g_warning ("glyph string length overflows maximum integer size, truncated");
-         new_len = string->space = G_MAXINT - 8;
+         const guint max_space =
+           MIN (G_MAXINT, G_MAXSIZE / MAX (sizeof(PangoGlyphInfo), sizeof(gint)));
+
+         guint more_space = (guint)string->space * 2;
+
+         if (more_space > max_space)
+           {
+             more_space = max_space;
+
+             if ((guint)new_len > max_space)
+               {
+                 g_error ("%s: failed to allocate glyph string of length %i\n",
+                          G_STRLOC, new_len);
+               }
+           }
+
+         string->space = more_space;
        }

Using this patch, the immediate multiplication by two was removed and checks for possible integer overflows where added in case of request for more space.

Written by xorl

May 8, 2009 at 11:19

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s