xorl %eax, %eax

CVE-2009-2463: Mozilla Firefox/Thunderbird Base64 Integer Overflows

with 6 comments

This bug was reported by monarch2020 and disclosed on 21 July 2009 by Mozilla. The issue affects specifically the Base64 routines in Mozilla Firefox prior to 3.0.12 release. This code can be found at nsprpub/lib/libc/src/base64.c of 3.0.11 release of Firefox like this.

/*
 * PL_Base64Encode
 *
 * If the destination argument is NULL, a return buffer is 
 * allocated, and the data therein will be null-terminated.  
 * If the destination argument is not NULL, it is assumed to
 * be of sufficient size, and the contents will not be null-
 * terminated by this routine.
 *
 * Returns null if the allocation fails.
 */

PR_IMPLEMENT(char *)
PL_Base64Encode
(
    const char *src,
    PRUint32    srclen,
    char       *dest
)
{
    if( 0 == srclen )
    {
        srclen = PL_strlen(src);
    }

    if( (char *)0 == dest )
    {
        PRUint32 destlen = ((srclen + 2)/3) * 4;
        dest = (char *)PR_MALLOC(destlen + 1);
        if( (char *)0 == dest )
        {
            return (char *)0;
        }
        dest[ destlen ] = (char)0; /* null terminate */
    }

    encode((const unsigned char *)src, srclen, (unsigned char *)dest);
    return dest;
}

So, the above encoding routine uses PL_strlen() to retrieve the length of ‘src’ string value. However, strlen() returns size_t which in some architectures is defined as unsigned long. This might lead to truncation on some 64-bit systems since ‘srclen’ is always 32-bit long, unsigned integer as you can read from the above routine. In addition to this, another bug is present in the second if clause. Here, the calculation of ‘destlen’ can easily result into an integer overflow and consequently to heap memory corruption. Both bugs were fixed by applying this patch:

     if( 0 == srclen )
     {
-        srclen = PL_strlen(src);
+        size_t len = strlen(src);
+        srclen = len;
+        /* Detect truncation. */
+        if( srclen != len )
+        {
+            return (char *)0;
+        }
     }
 
     if( (char *)0 == dest )
     {
-        PRUint32 destlen = ((srclen + 2)/3) * 4;
+        PRUint32 destlen;
+        /* Ensure all PRUint32 values stay within range. */
+        if( srclen > (PR_UINT32_MAX/4) * 3 )
+        {
+            return (char *)0;
+        }
+        destlen = ((srclen + 2)/3) * 4;
         dest = (char *)PR_MALLOC(destlen + 1);

Now, a temporary variable ‘len’ of size_t type is used to detect possible truncations in 32-bit long integer ‘srclen’, and the calculation in the second if is checked for overflows before proceeding to the allocation using PR_MALLOC(). Similar vulnerabilities were also present in Base64 decoding routine from nsprpub/lib/libc/src/base64.c.

Written by xorl

August 1, 2009 at 01:11

Posted in bugs

6 Responses

Subscribe to comments with RSS.

  1. It’s kinda hard to pass a >= 4Gb long string to strlen(), right? :-P As far as I can see, this is the only way to trigger this bug, which, personally speaking, is kinda unrealistic :-P

    huku

    August 2, 2009 at 17:19

  2. You’ll need something to make ((srclen + 2)/3) * 4 wrap around zero. Yes, this is some huge length returned by PL_strlen().

    xorl

    August 2, 2009 at 21:05

  3. I was about to say the same thing, very unrealistic.

    Btw, You got the CVE number wrong. Should be 2463.

    webDEViL

    August 3, 2009 at 15:29

  4. Thank you webDEViL. Fixed it.

    xorl

    August 3, 2009 at 17:27

  5. actually, in the majority of situations, the bug wouldn’t trigger just because it surpasses userland’s linear address limits (you need a string ~0xC0000000 long to wrap to zero). Not sure what are the normal limits for a process/thread in x64.

    pablo

    August 4, 2009 at 14:32

  6. You are all correct, I was not able to trigger it. Doing so is theoretically possibly but would require a 64bit.

    The only interesting thing not mentioned here is if you could trigger it exploitation would be easy. The base64 decode routine ignores invalid base64 values, allowing an attacker to control the exact amount of overflow.

    Hence I discarded it and sent it to the mozilla team.

    Monarch

    November 16, 2009 at 16:00


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