[Bug 209545] Heap overflows in an driver

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209545

            Bug ID: 209545
           Summary: Heap overflows in an driver
           Product: Base System
           Version: 11.0-CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: kern
          Assignee: freebsd-bugs at FreeBSD.org
          Reporter: cturt at hardenedbsd.org

The `an` driver doesn't perform bound checks on the lengths used by the
`AIROFLSHGCHR` and `AIROFLSHPCHR` commands implemented through
`SIOCGPRIVATE_0`, which leads to heap overflows from the `copyin` calls,
accessible by a privileged user only.

sys/dev/an/if_an.c:

static int
an_ioctl(struct ifnet *ifp, u_long command, caddr_t data)
{
        ...

        switch (command) {
        ...
        case SIOCGPRIVATE_0:            /* used by Cisco client utility */
                if ((error = priv_check(td, PRIV_DRIVER)))
                        goto out;
                error = copyin(ifr->ifr_data, &l_ioctl, sizeof(l_ioctl));
                if (error)
                        goto out;
                mode = l_ioctl.command;

                AN_LOCK(sc);
                if (mode >= AIROGCAP && mode <= AIROGSTATSD32) {
                        error = readrids(ifp, &l_ioctl);
                } else if (mode >= AIROPCAP && mode <= AIROPLEAPUSR) {
                        error = writerids(ifp, &l_ioctl);
                } else if (mode >= AIROFLSHRST && mode <= AIRORESTART) {
                        error = flashcard(ifp, &l_ioctl);
                } else {
                        error =-1;
                }
                AN_UNLOCK(sc);
                if (!error) {
                        /* copy out the updated command info */
                        error = copyout(&l_ioctl, ifr->ifr_data,
sizeof(l_ioctl));
                }
                break;
        ...
out:

        return(error != 0);
}       

static int
flashcard(struct ifnet *ifp, struct aironet_ioctl *l_ioctl)
{
        int             z = 0, status;
        struct an_softc *sc;

        sc = ifp->if_softc;
        if (sc->mpi350) {
                if_printf(ifp, "flashing not supported on MPI 350 yet\n");
                return(-1);
        }
        status = l_ioctl->command;

        switch (l_ioctl->command) {
        ...
        case AIROFLSHGCHR:      /* Get char from aux */
                AN_UNLOCK(sc);
                status = copyin(l_ioctl->data, &sc->areq, l_ioctl->len);
                AN_LOCK(sc);
                if (status)
                        return status;
                z = *(int *)&sc->areq;
                if ((status = flashgchar(ifp, z, 8000)) == 1)
                        return 0;
                else
                        return -1;
        case AIROFLSHPCHR:      /* Send char to card. */
                AN_UNLOCK(sc);
                status = copyin(l_ioctl->data, &sc->areq, l_ioctl->len);
                AN_LOCK(sc);
                if (status)
                        return status;
                z = *(int *)&sc->areq;
                if ((status = flashpchar(ifp, z, 8000)) == -1)
                        return -EIO;
                else
                        return 0;
                break;
        ...

        return -EINVAL;
}

I propose that the following bound checks are added to this command:

        case AIROFLSHGCHR:      /* Get char from aux */
+               if (l_ioctl->len > sizeof(sc->areq)) {
+                       return -EINVAL;
+               }
...
        case AIROFLSHPCHR:      /* Send char to card. */
+               if (l_ioctl->len > sizeof(sc->areq)) {
+                       return -EINVAL;
+               }

-- 
You are receiving this mail because:
You are the assignee for the bug.