Commit 340d51df authored by Laszlo Ersek's avatar Laszlo Ersek Committed by Michael Roth
Browse files

qga: escape cmdline args when registering win32 service (CVE-2013-2231) 



Reported-by: default avatarLev Veyde <lveyde@redhat.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Signed-off-by: default avatarLaszlo Ersek <lersek@redhat.com>
Signed-off-by: default avatarMichael Roth <mdroth@linux.vnet.ibm.com>
parent 108365fd

qga/service-win32.c

+74 −5
Original line number Diff line number Diff line
@@ -35,6 +35,68 @@ static int printf_win_error(const char *text) 
    return n;

}



/* Windows command line escaping. Based on

 * <http://blogs.msdn.com/b/oldnewthing/archive/2010/09/17/10063629.aspx> and

 * <http://msdn.microsoft.com/en-us/library/windows/desktop/17w5ykft%28v=vs.85%29.aspx>.

 *

 * The caller is responsible for initializing @buffer; prior contents are lost.

 */

static const char *win_escape_arg(const char *to_escape, GString *buffer)

{

    size_t backslash_count;

    const char *c;



    /* open with a double quote */

    g_string_assign(buffer, "\"");



    backslash_count = 0;

    for (c = to_escape; *c != '\0'; ++c) {

        switch (*c) {

        case '\\':

            /* The meaning depends on the first non-backslash character coming

             * up.

             */

            ++backslash_count;

            break;



        case '"':

            /* We must escape each pending backslash, then escape the double

             * quote. This creates a case of "odd number of backslashes [...]

             * followed by a double quotation mark".

             */

            while (backslash_count) {

                --backslash_count;

                g_string_append(buffer, "\\\\");

            }

            g_string_append(buffer, "\\\"");

            break;



        default:

            /* Any pending backslashes are without special meaning, flush them.

             * "Backslashes are interpreted literally, unless they immediately

             * precede a double quotation mark."

             */

            while (backslash_count) {

                --backslash_count;

                g_string_append_c(buffer, '\\');

            }

            g_string_append_c(buffer, *c);

        }

    }



    /* We're about to close with a double quote in string delimiter role.

     * Double all pending backslashes, creating a case of "even number of

     * backslashes [...] followed by a double quotation mark".

     */

    while (backslash_count) {

        --backslash_count;

        g_string_append(buffer, "\\\\");

    }

    g_string_append_c(buffer, '"');



    return buffer->str;

}



int ga_install_service(const char *path, const char *logfile,

                       const char *state_dir)

{
@@ -42,6 +104,7 @@ int ga_install_service(const char *path, const char *logfile, 
    SC_HANDLE manager;

    SC_HANDLE service;

    TCHAR module_fname[MAX_PATH];

    GString *esc;

    GString *cmdline;

    SERVICE_DESCRIPTION desc = { (char *)QGA_SERVICE_DESCRIPTION };
@@ -50,17 +113,22 @@ int ga_install_service(const char *path, const char *logfile, 
        return EXIT_FAILURE;

    }



    cmdline = g_string_new(module_fname);

    g_string_append(cmdline, " -d");

    esc     = g_string_new("");

    cmdline = g_string_new("");



    g_string_append_printf(cmdline, "%s -d",

                           win_escape_arg(module_fname, esc));



    if (path) {

        g_string_append_printf(cmdline, " -p %s", path);

        g_string_append_printf(cmdline, " -p %s", win_escape_arg(path, esc));

    }

    if (logfile) {

        g_string_append_printf(cmdline, " -l %s -v", logfile);

        g_string_append_printf(cmdline, " -l %s -v",

                               win_escape_arg(logfile, esc));

    }

    if (state_dir) {

        g_string_append_printf(cmdline, " -t %s", state_dir);

        g_string_append_printf(cmdline, " -t %s",

                               win_escape_arg(state_dir, esc));

    }



    g_debug("service's cmdline: %s", cmdline->str);
@@ -89,6 +157,7 @@ out_manager: 


out_strings:

    g_string_free(cmdline, TRUE);

    g_string_free(esc, TRUE);

    return ret;

}