Linux kernel IEEE 802.11 SoftMAC Off-by-One Overwrite

I just saw this in 2.6.30.4’s ChangeLog. The bug was reported by Dan Aloni and it can be found at drivers/staging/rtl8187se/ieee80211/ieee80211_softmac_wx.c. Here is the buggy routine from 2.6.30 release of the Linux kernel.

int ieee80211_wx_get_name(struct ieee80211_device *ieee,
                             struct iw_request_info *info,
                             union iwreq_data *wrqu, char *extra)
{
        strcpy(wrqu->name, "802.11");
        if(ieee->modulation & IEEE80211_CCK_MODULATION){
                strcat(wrqu->name, "b");
                if(ieee->modulation & IEEE80211_OFDM_MODULATION)
                        strcat(wrqu->name, "/g");
        }else if(ieee->modulation & IEEE80211_OFDM_MODULATION)
                strcat(wrqu->name, "g");

        if((ieee->state == IEEE80211_LINKED) ||
                (ieee->state == IEEE80211_LINKED_SCANNING))
                strcat(wrqu->name," linked");
        else if(ieee->state != IEEE80211_NOLINK)
                strcat(wrqu->name," link..");


        return 0;
}

It is a simple routine that constructs the name of the wireless device. As we can read at include/linux/wireless.h, structure iwreq_data is defined like this:

/* ------------------------ IOCTL REQUEST ------------------------ */
/*
 * This structure defines the payload of an ioctl, and is used 
 * below.
 *
 * Note that this structure should fit on the memory footprint
 * of iwreq (which is the same as ifreq), which mean a max size of
 * 16 octets = 128 bits. Warning, pointers might be 64 bits wide...
 * You should check this when increasing the structures defined
 * above in this file...
 */
union   iwreq_data
{
        /* Config - generic */
        char            name[IFNAMSIZ];
        /* Name : used to verify the presence of  wireless extensions.
         * Name of the protocol/provider... */

        struct iw_point essid;          /* Extended network name */
   ...
};

And constant IFNAMSIZ is simply 16 as we can read from include/linux/if.h. Now, if we move back to ieee80211_wx_get_name() we’ll see that there is a case as Dan Aloni found where the following addition might happen:

“802.11” ++ “b” ++ “/g” ++ ” linked” ++ “\x00”

This result in 17 bytes instead of 16 which is the size of ‘name’ array. The fix to this bug was to change the string routines with the equivalent strl functions like this:

 {
-	strcpy(wrqu->name, "802.11");
+	strlcpy(wrqu->name, "802.11", IFNAMSIZ);
 	if(ieee->modulation & IEEE80211_CCK_MODULATION){
-		strcat(wrqu->name, "b");
+		strlcat(wrqu->name, "b", IFNAMSIZ);
 		if(ieee->modulation & IEEE80211_OFDM_MODULATION)
-			strcat(wrqu->name, "/g");
+			strlcat(wrqu->name, "/g", IFNAMSIZ);
 	}else if(ieee->modulation & IEEE80211_OFDM_MODULATION)
-		strcat(wrqu->name, "g");
+		strlcat(wrqu->name, "g", IFNAMSIZ);
 
 	if((ieee->state == IEEE80211_LINKED) ||
 		(ieee->state == IEEE80211_LINKED_SCANNING))
-		strcat(wrqu->name," linked");
+		strlcat(wrqu->name,"  link", IFNAMSIZ);
 	else if(ieee->state != IEEE80211_NOLINK)
-		strcat(wrqu->name," link..");
+		strlcat(wrqu->name," .....", IFNAMSIZ);
 
 
 	return 0;