kern/126075: Network: internet control accesses beyond end of structure

[Steve Sears]

>Number:         126075
>Category:       kern
>Synopsis:       Network: internet control accesses beyond end of structure
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Jul 29 14:10:03 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator:     Steve Sears
>Release:        6.1, 7.0
>Organization:
NetApp
>Environment:
n/a
>Description:

In the kernel in sys/netinet/in.c: in_control() we have the following code, marked at the problem area:

     * If an alias address was specified, find that one instead of
     * the first one on the interface, if possible.
     */
    if (ifp) {
        dst = ((struct sockaddr_in *)&ifr->ifr_addr)->sin_addr;
        LIST_FOREACH(iap, INADDR_HASH(dst.s_addr), ia_hash)
            if (iap->ia_ifp == ifp &&
                iap->ia_addr.sin_addr.s_addr == dst.s_addr) {
                ia = iap;
                break;
            }
        if (ia == NULL)
            TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
->              iap = ifatoia(ifa);
->              if (iap->ia_addr.sin_family == AF_INET) {
->                  ia = iap;
->                  break;
                }
            }
    }

The macro ifatoia() is a simple cast:

#define ifatoia(ifa)    ((struct in_ifaddr *)(ifa))

Now look at the structure def for in_ifaddr:

struct in_ifaddr {
        struct  ifaddr ia_ifa;          /* protocol-independent info */
#define ia_ifp          ia_ifa.ifa_ifp
#define ia_flags        ia_ifa.ifa_flags
                                        /* ia_{,sub}net{,mask} in host order */
        u_long  ia_net;                 /* network number of interface */
        u_long  ia_netmask;             /* mask of net part */
        u_long  ia_subnet;              /* subnet number, including net */
        u_long  ia_subnetmask;          /* mask of subnet part */
        struct  in_addr ia_netbroadcast; /* to recognize net broadcasts */
        LIST_ENTRY(in_ifaddr) ia_hash;  /* entry in bucket of inet addresses */
        TAILQ_ENTRY(in_ifaddr) ia_link; /* list of internet addresses */
        struct  sockaddr_in ia_addr;    /* reserve space for interface name */
        struct  sockaddr_in ia_dstaddr; /* reserve space for broadcast addr */
#define ia_broadaddr    ia_dstaddr
        struct  sockaddr_in ia_sockmask; /* reserve space for general netmask */
};

All of the internet types include an ifaddr at the beginning, but it accessing beyond this struct after a cast is not a safe operation.


>How-To-Repeat:
Use non AF_INET network interfaces.
>Fix:
The problem is that ia_addr is not part of the overloaded ifaddr structure, so this check falls off the end of the structure in all but the in_ifaddr case. The corrected code should be:

        if (ia == NULL)
            TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
+               if (ifa->ifa_addr->sa_family == AF_INET) {
+                   ia = ifatoia(ifa);
+                   break;
+               }

This prevents bogus data being returned due to a cast allowing the code to walk off the end of valid data.



>Release-Note:
>Audit-Trail:
>Unformatted: