[Qemu-devel] [PATCH 3/3] qga: escape cmdline args when registering win32 service (CVE-2013-2231)

[Michael Roth]

From: Laszlo Ersek <address@hidden>

Reported-by: Lev Veyde <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Laszlo Ersek <address@hidden>
Signed-off-by: Michael Roth <address@hidden>
---
 qga/service-win32.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 74 insertions(+), 5 deletions(-)

diff --git a/qga/service-win32.c b/qga/service-win32.c
index 1cf81fc..aef41f0 100644
--- a/qga/service-win32.c
+++ b/qga/service-win32.c
@@ -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;
 }
 
-- 
1.7.9.5