aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLaszlo Ersek <lersek@redhat.com>2013-07-03 15:14:10 +0200
committerMichael Roth <mdroth@linux.vnet.ibm.com>2013-07-23 12:02:47 -0500
commitbe161ae35df74b24f090ad00913442f0b110c1bc (patch)
tree98cc84e55b538e181a102417b4a6dd35fe7fb8b1
parentbb31546dfa56a3a5812d4bd1016f262904766915 (diff)
qga: escape cmdline args when registering win32 service (CVE-2013-2231)
Reported-by: Lev Veyde <lveyde@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
-rw-r--r--qga/service-win32.c79
1 files changed, 74 insertions, 5 deletions
diff --git a/qga/service-win32.c b/qga/service-win32.c
index 1cf81fc96d..aef41f04f1 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;
}