ip_dummynet_compat improve size validation

[ecturt_gmail.com]

Summary

ip_dummynet_compat doesn't validate sopt_valsize correctly.

The IP_DUMMYNET_DEL and IP_DUMMYNET_CONFIGURE commands allocate and copyin using this size, and then pass the buffer onto dn_compat_del and dn_compat_configure respectively. There are several issues with this.

Passing sizes less than the sizes of the expected structs, for example 0, would result in allocating and copying 0 bytes, and then passing this buffer onto these functions, where it is dereferenced as a structure of a different size, leading to out of bounds heap access.

As a side note, is7 is a global variable, and so this code can be raced. A process could sabotage another process' setsockopt call by making its own setsockopt request to change the is7 value whilst the other setsockopt call is in mid-execution. An easy fix for this would be to take a lock around ip_dummynet_compat calls, however it would potentially be cleaner to refactor this code to pass the is7 value as an argument to function calls.

int
ip_dummynet_compat(struct sockopt *sopt)
{
    int error=0;
    void *v = NULL;
    struct dn_id oid;

    /* Length of data, used to found ipfw version... */
    int len = sopt->sopt_valsize;

    /* len can be 0 if command was dummynet_flush */
    if (len == pipesize7) {
        D("setting compatibility with FreeBSD 7.2");
        is7 = 1;
    }
    else if (len == pipesize8 || len == pipesizemax8) {
        D("setting compatibility with FreeBSD 8");
        is7 = 0;
    }

    switch (sopt->sopt_name) {
    ...
    case IP_DUMMYNET_DEL:
        v = malloc(len, M_TEMP, M_WAITOK);
        error = sooptcopyin(sopt, v, len, len);
        if (error)
            break;
        error = dn_compat_del(v);
        free(v, M_TEMP);
        break;

    case IP_DUMMYNET_CONFIGURE:
        v = malloc(len, M_TEMP, M_WAITOK);
        error = sooptcopyin(sopt, v, len, len);
        if (error)
            break;
        error = dn_compat_configure(v);
        free(v, M_TEMP);
        break;
    ...

The fix for this issue is to return EINVAL for unexpected sizes.