xorl %eax, %eax

CVE-2009-4593: bftpd Remote off-by-one

leave a comment »

This vulnerability was reported by Dazhi as we can read from the official site. The issue affects bftpd prior to 2.4 release, here is the buggy code that resides in bftpdutmp.c of 2.3 release…

void bftpdutmp_log(char type)
{
    struct bftpdutmp ut, tmp;
    long i;
     ...
    memset((void *) &ut, 0, sizeof(ut));
    ut.bu_pid = getpid();
    if (type) {
        ut.bu_type = 1;
        strncpy(ut.bu_name, user, sizeof(ut.bu_name));
        strncpy(ut.bu_host, remotehostname, sizeof(ut.bu_host));
       /* Determine offset of first user marked dead */
        rewind(bftpdutmp);
        i = 0;
        while (fread((void *) &tmp, sizeof(tmp), 1, bftpdutmp)) {
            if (!tmp.bu_type)
                break;
            i++;
        }
        bftpdutmp_offset = i * sizeof(tmp);
    }
     ...
    fflush(bftpdutmp);
}

This routine is responsible for UTMP logging. As you can read, after zeroing out the ‘ut’ structure, it starts initializing its members. From the equivalent header file (bftpdutmp.h) we can read the definition of this structure…

struct bftpdutmp {
    char bu_type;
    pid_t bu_pid;
    char bu_name[USERLEN + 1];
    char bu_host[256];
    time_t bu_time;
};

After initializing the process ID field with the current process’ ID, it checks if the type passed to it as an argument is non-zero. If this is the case, it will assign the ‘type’ value to the equivalent member of the ‘bftpdutmp’ structure and then use strncpy(3) library routine to copy the username and the remote hostname to ‘bu_name’ and ‘bu_host’ respectively. If the remote hostname is equal, or longer than 256 characters, strncpy(3) will not have sufficient space to add the NULL termination and because of this, the ‘bu_host’ array will seem like it ends at the NULL termination of ‘bu_name’ buffer. Because of this, processing of this buffer results in invalid operations. To fix this simple vulnerability, the following patch was applied:

         strncpy(ut.bu_name, user, sizeof(ut.bu_name));
-        strncpy(ut.bu_host, remotehostname, sizeof(ut.bu_host));
+        max_length = sizeof(ut.bu_host);
+        strncpy(ut.bu_host, remotehostname, max_length);
+        ut.bu_host[max_length - 1] = '\0';     /* avoid non-null terminated string */
        /* Determine offset of first user marked dead */
         rewind(bftpdutmp);

Where ‘max_length’ is a new variable added to this function as you can see below:

     struct bftpdutmp ut, tmp;
     long i;
+    int max_length;
+
     if (!bftpdutmp)

The patch determines the size of the ‘bu_host’ array and after the strncpy(3) it manually NULL terminates the string.

Written by xorl

January 11, 2010 at 19:51

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