xorl %eax, %eax

CVE-2010-2642: Evince AFM Font Parser Heap Memory Corruption

with 4 comments

In addition to CVE-2010-2643, this issue was also reported by Jon Larimer of IBM X-Force. This time the bug affects the AFM (Adobe Font Metrics) file format parser. The suscpetible code is available at backend/dvi/mdvi-lib/afmparse.c and more specifically in the following routine.

static char *ident = NULL; /* storage buffer for keywords */
  ...
/*  A "AFM File Conventions" tokenizer. That means that it will
 *  return the next token delimited by white space.  See also
 *  the `linetoken' routine, which does a similar thing but 
 *  reads all tokens until the next end-of-line.
 */
 
static char *token(FILE *stream)
{
    int ch, idx;
  ...
    idx = 0;
    while (ch != EOF && ch != ' ' && ch != lineterm 
           && ch != '\t' && ch != ':' && ch != ';') 
    {
        ident[idx++] = ch;
        ch = fgetc(stream);
    } /* while */
  ...
    return(ident);      /* returns pointer to the token */

} /* token */

The ‘while’ loop will copy the file’s data to the global ‘ident’ array based on these conditions:
– It has not reached the end of file (EOF)
– It’s not processing a space character
– It has not reached the line termination
– The current character isn’t a horizontal tab
– The current character isn’t a colon or a semi-colon
This seems to be fine but a quick look at the first routine that is being called in this parser reveals that the ‘ident’ pointer is initialized with a static size of ‘MAX_NAME’.

extern int afm_parse_file(FILE *fp, FontInfo **fi, FLAGS flags)
{
    
    int code = ok;      /* return code from each of the parsing routines */
    int error = ok;     /* used as the return code from this function */
    
    register char *keyword; /* used to store a token */  
    
                              
    /* storage data for the global variable ident */                          
    ident = (char *) calloc(MAX_NAME, sizeof(char)); 
    if (ident == NULL) {error = storageProblem; return(error);}   
  ...
} /* parseFile */

Which according to the equivalent header file located at backend/dvi/mdvi-lib/afmparse.h is…

#define MAX_NAME 4096           /* max length for identifiers */

Back to token() we can see that there is no check in the copying loop for maximum size meaning that it could easily result in heap memory corruption. Clearly, the patch to fix this bug was to add the missing check as shown below.

    idx = 0;
    while (ch != EOF && ch != ' ' && ch != lineterm 
-          && ch != '\t' && ch != ':' && ch != ';') 
+          && ch != '\t' && ch != ':' && ch != ';' && idx < MAX_NAME)
    {

That also checks that the index value ‘idx’ is within the ‘MAX_NAME’ range.

Written by xorl

January 14, 2011 at 14:20

Posted in bugs

4 Responses

Subscribe to comments with RSS.

  1. What about linetoken() in the same file? It also seems affected.

    non-customers

    January 15, 2011 at 19:27

  2. Indeed. And according to their GIT tree I just checked [1] it’s still vulnerable.

    Dear non-customers you’ve just killed another possible 0day…

    [1] http://git.gnome.org/browse/evince/tree/backend/dvi/mdvi-lib/afmparse.c#n186

    xorl

    January 15, 2011 at 20:17

  3. Perhaps I am missing something – but it looks like the fix contains a problem, i.e. for

    idx=4095

    => idx<MAX_NAME is true

    and if a character is written at that position, then the string is not null-terminated anymore.

    georg

    March 23, 2011 at 22:14

  4. It seems to me that you’re right.

    xorl

    March 25, 2011 at 15:16


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