xorl %eax, %eax

CVE-2011-1080: Linux kernel NetFilter Missing NULL Termination

leave a comment »

Another bug reported by Vasiliy Kulikov, this time in NetFilter’s code. More specifically in net/bridge/netfilter/ebtables.c we can find the following.

/* replace the table */
static int do_replace(struct net *net, const void __user *user,
                      unsigned int len)
{
        int ret, countersize;
        struct ebt_table_info *newinfo;
        struct ebt_replace tmp;

        if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
                return -EFAULT;
  ...
free_newinfo:
        vfree(newinfo);
        return ret;
}

Here we have the ‘ebt_replace’ structure being copied from user space. This structure is defined in include/linux/netfilter_bridge/ebtables.h header file and looks like this.

#define EBT_TABLE_MAXNAMELEN 32
  ...
struct ebt_replace {
        char name[EBT_TABLE_MAXNAMELEN];
        unsigned int valid_hooks;
        /* nr of rules in the table */
        unsigned int nentries;
        /* total size of the entries */
        unsigned int entries_size;
        /* start of the chains */
        struct ebt_entries __user *hook_entry[NF_BR_NUMHOOKS];
        /* nr of counters userspace expects back */
        unsigned int num_counters;
        /* where the kernel will put the old counters */
        struct ebt_counter __user *counters;
        char __user *entries;
};

A user can provide a table name that will be non-NULL terminated. Next, the below functions will attempt to use the appropriate module based on the given name.

static void *
find_inlist_lock(struct list_head *head, const char *name, const char *prefix,
   int *error, struct mutex *mutex)
{
        return try_then_request_module(
                        find_inlist_lock_noload(head, name, error, mutex),
                        "%s%s", prefix, name);
}
  ...
static int compat_mtw_from_user(struct compat_ebt_entry_mwt *mwt,
                                enum compat_mwt compat_mwt,
                                struct ebt_entries_buf_state *state,
                                const unsigned char *base)
{
  ...
        case EBT_COMPAT_MATCH:
                match = try_then_request_module(xt_find_match(NFPROTO_BRIDGE,
                                                name, 0), "ebt_%s", name);
  ...
        case EBT_COMPAT_WATCHER: /* fallthrough */
        case EBT_COMPAT_TARGET:
                wt = try_then_request_module(xt_find_target(NFPROTO_BRIDGE,
                                                name, 0), "ebt_%s", name);
  ...
        return off + match_size;
}

So, as Vasiliy Kulikov noticed, the non-NULL terminated name string would be viewable by all user-space processes as a module name.

The fix was to NULL temrinate the name.

 	if (tmp.num_counters >= INT_MAX / sizeof(struct ebt_counter))
 		return -ENOMEM;
 
+	tmp.name[sizeof(tmp.name)-1] = 0;
+
 	countersize = COUNTER_OFFSET(tmp.nentries) * nr_cpu_ids;

Written by xorl

May 3, 2011 at 20:01

Posted in bugs, linux

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