CVE-2009-1364: LibWMF Pointer Use after free()

This is an open source library for Windows MetaFile (WMF) file format which you can find at this site. This vulnerability was reported by Tavis Ormandy of the Google Security Team and affects libwmf 0.2.8.4 release. This library is used in numerous GNU/Linux distributions by default. Here is the buggy code:

54 void gdClipSetAdd(gdImagePtr im,gdClipRectanglePtr rect)
55 {       gdClipRectanglePtr more;
56
57         if (im->clip == 0)
       ...
68         }
69         if (im->clip->count == im->clip->max)
70         {       more = gdRealloc (im->clip->list,(im->clip->max + 8) * 
70                                   sizeof (gdClipRectangle));
71                 if (more == 0) return;
72                 im->clip->max += 8;
73         }
74         im->clip->list[im->clip->count] = (*rect);
75         im->clip->count++;
76 }

You can find this function at src/extra/gd/gd_clip.c which is the internal GD library used in various operations. The above gdImagePtr can be found in gd.h:

typedef gdImage * gdImagePtr;

And gdImage is a structure with a lot of members, among them:

73 typedef struct gdImageStruct {
74         /* Palette-based image pixels */
75         unsigned char ** pixels;
       ...
133         gdClipSet* clip; /* See <gd_clip.h> */
134         int * _tpixels; /* contiguous pixel array */
135 } gdImage;

And gdClipSet from gd_clip.h is:

14 /* ClipSet type */
15 typedef struct {
16         int max;
17         int count;
18
19         gdClipRectangle* list;
20 } gdClipSet;

Now, by having a look at gdClipSetAdd() we can see that if the passed ClipSet is not zero and its ClipSet counter is equal to the maximum value (line 69), it will invoke gdRealloc() passing the current position of list pointer, in order to allocate:

(im->clip->max + 8) * sizeof(gdClipRectangle)

additional bytes. If gdRealloc() returns 0 (line 71) the function will terminate. In any other case, it updates the list by adding the new rectangle to the new index at line 74 and then increments the counter/index. The flaw is quiet obvious. The author’s intention was to use more as the new pointer, however he forgets to set im->clip->list to the value of more. By doing this, the write operation at line 74 will write data to a pointer that does not necessarily point to the same location as the one returned by gdRealloc() which as we can see at gdhelpers.c does this:

86 void *
87 gdRealloc (void *ptr, size_t size)
88 {
89   return realloc (ptr, size);
90 }

Just a wrapper around realloc(3). A quick look at its man page…

realloc()  changes  the  size  of the memory block 
pointed to by ptr to size bytes.
        ...
If the area pointed to was moved, a free(ptr) is done.

The above code will fail if realloc(3) moves the allocated memory to another location and free(3)s im->clip->list which is used at line 74 instead of more. This is not patched yet, even though the fix is fairly simple…

                im->clip->max += 8;
+               im->clip->list = more;
        }
        im->clip->list[im->clip->count] = (*rect);