kern/44054: Stacksmashing in handling of ioctl OOSIO* parameter

[o.vd.linden%quicknet.nl@localhost]

>Number:         44054
>Category:       kern
>Synopsis:       Stacksmashing in handling of ioctl OOSIO* parameter
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Nov 06 11:25:00 +0000 2010
>Originator:     Onno van der Linden
>Release:        NetBSD/i386 5.99.39
>Organization:
>Environment:
NetBSD sheep 5.99.39 NetBSD 5.99.39 (SHEEP) #35: Sat Nov  6 11:40:11 MET 2010  
onno@sheep:/usr/src/sys/arch/i386/compile/SHEEP i386
>Description:
For ioctl parameters OOSIOCGIFADDR, OOSIOCGIFDSTADDR, OOSIOCGIFBRDADDR and 
OOSIOCGIFNETMASK a  wrongly sized data buffer is passed on the stack to
ifreq_setaddr() where the call to memset() will smash the stack.
>How-To-Repeat:
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <net/if.h>
#include <netinet/in.h>

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

struct oifreq {
        char    ifr_name[IFNAMSIZ];             /* if name, e.g. "en0" */
        union {
                struct  sockaddr ifru_addr;
                struct  sockaddr ifru_dstaddr;
                struct  sockaddr ifru_broadaddr;
                short   ifru_flags;  
                int     ifru_metric;
                int     ifru_mtu; 
                int     ifru_dlt;
                u_int   ifru_value;
                void *  ifru_data;
                struct {
                        uint32_t        b_buflen;
                        void            *b_buf;
                } ifru_b;
        } ifr_ifru;
};      


#define OOSIOCGIFBRDADDR _IOWR('i', 18, struct oifreq)

main()
{
        int     fd;
        struct oifreq   ifreq;
        struct sockaddr_in      *sin;

        memset(&ifreq,'\0',sizeof ifreq);
        strcpy(ifreq.ifr_name,"wm0");

        fd = socket(AF_INET, SOCK_DGRAM, 0);
        if (fd < 0) {
                perror("socket");
                exit(1);
        }

        if (ioctl(fd, OOSIOCGIFBRDADDR, &ifreq) < 0) {
                perror("OOSIOCGIFBRDADDR");
                exit(1);
        }
        sin = (struct sockaddr_in *)&ifreq.ifr_broadaddr;
        printf("broadcast: %s\n", inet_ntoa(sin->sin_addr));

        close(fd);
}

>Fix:
compat_cvtcmd() doesn't translate the OOSIO parameter into its SIO counterpart,
there's some special handling in compat_ifioctl() already which could be used
to pass a rightly sized buffer, like the way the OSIO parameters are handled in 
ifioctl().

Another possible fix is to put a case statement in compat_cvtcmd() in place of 
the
ncmd assignment where the OOSIO parameters (and others ?) are handled as
seperate cases and in which the default case gets the ncmd assignment. But 
that probably needs some additional changes to compat_ifioctl() too.