[Bug 208808] Heap overflow in nlm system call

[bugzilla-noreply at freebsd.org]

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

            Bug ID: 208808
           Summary: Heap overflow in nlm system call
           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

There is a kernel heap overflow in the `nlm_register_services` function,
reachable through the `nlm` system call. Due to the `priv_check` call in
`sys_nlm_syscall`, this is triggerable as root only, so is non critical.

`nlm_register_services` is reachable with user controlled `addr_count`, where
the following call to `malloc` is performed:

    xprts = malloc(addr_count * sizeof(SVCXPRT *), M_NLM, M_WAITOK|M_ZERO);

Firstly, since this allocation passes the `M_WAITOK` flag, by specifying large
enough sizes, a user could cause DoS. However, more interestingly, by
specifying an `addr_count` such that integer overflow occurs during the
multiplication, an undersized buffer would be allocated, leading to heap
overflow later on.

Full code path goes through `sys_nlm_syscall -> nlm_server_main ->
nlm_register_services`, `(sys/nlm/nlm_prot_impl.c)`:

int
sys_nlm_syscall(struct thread *td, struct nlm_syscall_args *uap)
{
        int error;

#if __FreeBSD_version >= 700000
        error = priv_check(td, PRIV_NFS_LOCKD);
#else
        error = suser(td);
#endif
        if (error)
                return (error);

        nlm_debug_level = uap->debug_level;
        nlm_grace_threshold = time_uptime + uap->grace_period;
        nlm_next_idle_check = time_uptime + NLM_IDLE_PERIOD;

        return nlm_server_main(uap->addr_count, uap->addrs);
}

static int
nlm_server_main(int addr_count, char **addrs)
{
        struct thread *td = curthread;
        int error;
        SVCPOOL *pool = NULL;
        struct sockopt opt;
        int portlow;
#ifdef INET6
        struct sockaddr_in6 sin6;
#endif
        struct sockaddr_in sin;
        my_id id;
        sm_stat smstat;
        struct timeval timo;
        enum clnt_stat stat;
        struct nlm_host *host, *nhost;
        struct nlm_waiting_lock *nw;
        vop_advlock_t *old_nfs_advlock;
        vop_reclaim_t *old_nfs_reclaim;

        if (nlm_is_running != 0) {
                NLM_ERR("NLM: can't start server - "
                    "it appears to be running already\n");
                return (EPERM);
        }

        ...

        error = nlm_register_services(pool, addr_count, addrs);

        ...
}

static int
nlm_register_services(SVCPOOL *pool, int addr_count, char **addrs)
{
        static rpcvers_t versions[] = {
                NLM_SM, NLM_VERS, NLM_VERSX, NLM_VERS4
        };
        static void (*dispatchers[])(struct svc_req *, SVCXPRT *) = {
                nlm_prog_0, nlm_prog_1, nlm_prog_3, nlm_prog_4
        };
        static const int version_count = sizeof(versions) /
sizeof(versions[0]);

        SVCXPRT **xprts;
        char netid[16];
        char uaddr[128];
        struct netconfig *nconf;
        int i, j, error;

        if (!addr_count) {
                NLM_ERR("NLM: no service addresses given - can't start
server");
                return (EINVAL);
        }

        xprts = malloc(addr_count * sizeof(SVCXPRT *), M_NLM, M_WAITOK|M_ZERO);

        ...
}

I propose that there should be a bound check on `addr_count` in
`nlm_register_services`, such as the following:

        if (!addr_count) {
                NLM_ERR("NLM: no service addresses given - can't start
server");
                return (EINVAL);
        }

+       if (addr_count < 0 || addr_count > 256) {
+               NLM_ERR("NLM: too many service addresses given - can't start
server");
+               return (EINVAL);
+       }
+
        xprts = malloc(addr_count * sizeof(SVCXPRT *), M_NLM, M_WAITOK|M_ZERO);

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