Fix the conhost command line not being properly escaped (#1815)

This commit re-escapes the path to conhost's subprocess before it launches it.
microsoft · Jul 12, 2019 · 02e8389 · 02e8389
1 parent 6d3001f
commit 02e8389
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 3 deletions.
diff --git a/src/host/ConsoleArguments.cpp b/src/host/ConsoleArguments.cpp
@@ -21,6 +21,79 @@ const std::wstring_view ConsoleArguments::INHERIT_CURSOR_ARG = L"--inheritcursor
 const std::wstring_view ConsoleArguments::FEATURE_ARG = L"--feature";
 const std::wstring_view ConsoleArguments::FEATURE_PTY_ARG = L"pty";
 
+std::wstring EscapeArgument(std::wstring_view ac)
+{
+    if (ac.empty())
+    {
+        return L"\"\"";
+    }
+    bool hasSpace = false;
+    auto n = ac.size();
+    for (auto c : ac)
+    {
+        switch (c)
+        {
+        case L'"':
+        case L'\\':
+            n++;
+            break;
+        case ' ':
+        case '\t':
+            hasSpace = true;
+            break;
+        default:
+            break;
+        }
+    }
+    if (hasSpace)
+    {
+        n += 2;
+    }
+    if (n == ac.size())
+    {
+        return std::wstring{ ac };
+    }
+    std::wstring buf;
+    if (hasSpace)
+    {
+        buf.push_back(L'"');
+    }
+    size_t slashes = 0;
+    for (auto c : ac)
+    {
+        switch (c)
+        {
+        case L'\\':
+            slashes++;
+            buf.push_back(L'\\');
+            break;
+        case L'"':
+        {
+            for (; slashes > 0; slashes--)
+            {
+                buf.push_back(L'\\');
+            }
+            buf.push_back(L'\\');
+            buf.push_back(c);
+        }
+        break;
+        default:
+            slashes = 0;
+            buf.push_back(c);
+            break;
+        }
+    }
+    if (hasSpace)
+    {
+        for (; slashes > 0; slashes--)
+        {
+            buf.push_back(L'\\');
+        }
+        buf.push_back(L'"');
+    }
+    return buf;
+}
+
 ConsoleArguments::ConsoleArguments(const std::wstring& commandline,
                                    const HANDLE hStdIn,
                                    const HANDLE hStdOut) :
@@ -272,7 +345,7 @@ void ConsoleArguments::s_ConsumeArg(_Inout_ std::vector<std::wstring>& args, _In
     size_t j = 0;
     for (j = index; j < args.size(); j++)
     {
-        _clientCommandline += args[j];
+        _clientCommandline += EscapeArgument(args[j]); // escape commandline
         if (j + 1 < args.size())
         {
             _clientCommandline += L" ";

diff --git a/src/host/ut_host/ConsoleArgumentsTests.cpp b/src/host/ut_host/ConsoleArgumentsTests.cpp
@@ -90,7 +90,7 @@ void ConsoleArgumentsTests::ArgSplittingTests()
                    INVALID_HANDLE_VALUE,
                    INVALID_HANDLE_VALUE,
                    ConsoleArguments(commandline,
-                                    L"this is the commandline", // clientCommandLine
+                                    L"\"this is the commandline\"", // clientCommandLine
                                     INVALID_HANDLE_VALUE,
                                     INVALID_HANDLE_VALUE,
                                     L"", // vtMode
@@ -110,7 +110,7 @@ void ConsoleArgumentsTests::ArgSplittingTests()
                    INVALID_HANDLE_VALUE,
                    INVALID_HANDLE_VALUE,
                    ConsoleArguments(commandline,
-                                    L"--vtmode bar this is the commandline", // clientCommandLine
+                                    L"\"--vtmode bar this is the commandline\"", // clientCommandLine
                                     INVALID_HANDLE_VALUE,
                                     INVALID_HANDLE_VALUE,
                                     L"", // vtMode
@@ -223,6 +223,26 @@ void ConsoleArgumentsTests::ArgSplittingTests()
                                     0, // signalHandle
                                     false), // inheritCursor
                    true); // successful parse?
+
+    commandline = L"conhost.exe this is the commandline";
+    ArgTestsRunner(L"#9 commandline no quotes",
+                   commandline,
+                   INVALID_HANDLE_VALUE,
+                   INVALID_HANDLE_VALUE,
+                   ConsoleArguments(commandline,
+                                    L"this is the commandline", // clientCommandLine
+                                    INVALID_HANDLE_VALUE,
+                                    INVALID_HANDLE_VALUE,
+                                    L"", // vtMode
+                                    0, // width
+                                    0, // height
+                                    false, // forceV1
+                                    false, // headless
+                                    true, // createServerHandle
+                                    0, // serverHandle
+                                    0, // signalHandle
+                                    false), // inheritCursor
+                   true); // successful parse?
 }
 
 void ConsoleArgumentsTests::ClientCommandlineTests()