Fix potential double-free() in Interactive Service (CVE-2018-9336)

Malformed input data on the service pipe towards the OpenVPN interactive
service (normally used by the OpenVPN GUI to request openvpn instances
from the service) can result in a double free() in the error handling code.

This usually only leads to a process crash (DoS by an unprivileged local
account) but since it could possibly lead to memory corruption if
happening while multiple other threads are active at the same time,
CVE-2018-9336 has been assigned to acknowledge this risk.

Fix by ensuring that sud->directory is set to NULL in GetStartUpData()
for all error cases (thus not being free()ed in FreeStartupData()).

Rewrite control flow to use explicit error label for error exit.

Discovered and reported by Jacob Baines <jbaines@tenable.com>.

CVE: 2018-9336

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20180414072617.25075-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20180414072617.25075-1-gert@greenie.muc.de

Signed-off-by: Gert Doering <gert@greenie.muc.de>
ValidateOptions(HANDLE pipe, const WCHAR *workdir, const WCHAR *options)
{
    WCHAR **argv;
    int argc;
    WCHAR buf[256];
    BOOL ret = FALSE;
    int i;
    const WCHAR *msg1 = L"You have specified a config file location (%s relative to %s)"
                        L" that requires admin approval. This error may be avoided"
                        L" by adding your account to the \"%s\" group";

    const WCHAR *msg2 = L"You have specified an option (%s) that may be used"
                        L" only with admin approval. This error may be avoided"
                        L" by adding your account to the \"%s\" group";

    argv = CommandLineToArgvW(options, &argc);

    if (!argv)
    {
        ReturnLastError(pipe, L"CommandLineToArgvW");
        ReturnError(pipe, ERROR_STARTUP_DATA, L"Cannot validate options", 1, &exit_event);
        goto out;
    }

    /* Note: argv[0] is the first option */
    if (argc < 1)  /* no options */
    {
        ret = TRUE;
        goto out;
    }

    /*
     * If only one argument, it is the config file
     */
    if (argc == 1)
    {
        WCHAR *argv_tmp[2] = { L"--config", argv[0] };

        if (!CheckOption(workdir, 2, argv_tmp, &settings))
        {
            swprintf(buf, _countof(buf), msg1, argv[0], workdir,
                     settings.ovpn_admin_group);
            buf[_countof(buf) - 1] = L'\0';
            ReturnError(pipe, ERROR_STARTUP_DATA, buf, 1, &exit_event);
        }
        goto out;
    }

    for (i = 0; i < argc; ++i)
    {
        if (!IsOption(argv[i]))
        {
            continue;
        }

        if (!CheckOption(workdir, argc-i, &argv[i], &settings))
        {
            if (wcscmp(L"--config", argv[i]) == 0 && argc-i > 1)
            {
                swprintf(buf, _countof(buf), msg1, argv[i+1], workdir,
                         settings.ovpn_admin_group);
                buf[_countof(buf) - 1] = L'\0';
                ReturnError(pipe, ERROR_STARTUP_DATA, buf, 1, &exit_event);
            }
            else
            {
                swprintf(buf, _countof(buf), msg2, argv[i],
                         settings.ovpn_admin_group);
                buf[_countof(buf) - 1] = L'\0';
                ReturnError(pipe, ERROR_STARTUP_DATA, buf, 1, &exit_event);
            }
            goto out;
        }
    }

    /* all options passed */
    ret = TRUE;

out:
    if (argv)
    {
        LocalFree(argv);
    }
    return ret;
}

ValidateOptions(HANDLE pipe, const WCHAR *workdir, const WCHAR *options)
{
    WCHAR **argv;
    int argc;
    WCHAR buf[256];
    BOOL ret = FALSE;
    int i;
    const WCHAR *msg1 = L"You have specified a config file location (%s relative to %s)"
                        L" that requires admin approval. This error may be avoided"
                        L" by adding your account to the \"%s\" group";

    const WCHAR *msg2 = L"You have specified an option (%s) that may be used"
                        L" only with admin approval. This error may be avoided"
                        L" by adding your account to the \"%s\" group";

    argv = CommandLineToArgvW(options, &argc);

    if (!argv)
    {
        ReturnLastError(pipe, L"CommandLineToArgvW");
        ReturnError(pipe, ERROR_STARTUP_DATA, L"Cannot validate options", 1, &exit_event);
        goto out;
    }

    /* Note: argv[0] is the first option */
    if (argc < 1)  /* no options */
    {
        ret = TRUE;
        goto out;
    }

    /*
     * If only one argument, it is the config file
     */
    if (argc == 1)
    {
        WCHAR *argv_tmp[2] = { L"--config", argv[0] };

        if (!CheckOption(workdir, 2, argv_tmp, &settings))
        {
            swprintf(buf, _countof(buf), msg1, argv[0], workdir,
                     settings.ovpn_admin_group);
            buf[_countof(buf) - 1] = L'\0';
            ReturnError(pipe, ERROR_STARTUP_DATA, buf, 1, &exit_event);
        }
        goto out;
    }

    for (i = 0; i < argc; ++i)
    {
        if (!IsOption(argv[i]))
        {
            continue;
        }

        if (!CheckOption(workdir, argc-i, &argv[i], &settings))
        {
            if (wcscmp(L"--config", argv[i]) == 0 && argc-i > 1)
            {
                swprintf(buf, _countof(buf), msg1, argv[i+1], workdir,
                         settings.ovpn_admin_group);
                buf[_countof(buf) - 1] = L'\0';
                ReturnError(pipe, ERROR_STARTUP_DATA, buf, 1, &exit_event);
            }
            else
            {
                swprintf(buf, _countof(buf), msg2, argv[i],
                         settings.ovpn_admin_group);
                buf[_countof(buf) - 1] = L'\0';
                ReturnError(pipe, ERROR_STARTUP_DATA, buf, 1, &exit_event);
            }
            goto out;
        }
    }

    /* all options passed */
    ret = TRUE;

out:
    if (argv)
    {
        LocalFree(argv);
    }
    return ret;
}
