launcher: ptrans: fix vulnerability reported by Anton from RIA Labs

src/launcher/launcher.c

@@ -504,18 +504,18 @@ static SVC_INSTANCE *SvcInstanceLookup(PWSTR ClassName, PWSTR InstanceName)
 
 static inline ULONG SvcInstanceArgumentLength(PWSTR Arg, PWSTR Pattern, BOOLEAN Quote)
 {
-    PWSTR PathTransform(PWSTR Dest, PWSTR Arg, PWSTR Pattern);
+    PWSTR PathTransform(PWSTR Dest, PWSTR Arg, PWSTR Pattern, BOOLEAN Quote);
 
-    return (Quote ? 2 : 0) + (ULONG)((UINT_PTR)PathTransform(0, Arg, Pattern) / sizeof(WCHAR));
+    return (Quote ? 2 : 0) + (ULONG)((UINT_PTR)PathTransform(0, Arg, Pattern, Quote) / sizeof(WCHAR));
 }
 
 static inline PWSTR SvcInstanceArgumentCopy(PWSTR Dest, PWSTR Arg, PWSTR Pattern, BOOLEAN Quote)
 {
-    PWSTR PathTransform(PWSTR Dest, PWSTR Arg, PWSTR Pattern);
+    PWSTR PathTransform(PWSTR Dest, PWSTR Arg, PWSTR Pattern, BOOLEAN Quote);
 
     if (Quote)
         *Dest++ = L'"';
-    Dest = PathTransform(Dest, Arg, Pattern);
+    Dest = PathTransform(Dest, Arg, Pattern, Quote);
     if (Quote)
         *Dest++ = L'"';
 

src/launcher/ptrans.c

@@ -64,8 +64,11 @@
 #include <winfsp/launch.h>
 #include <shared/um/minimal.h>
 
-static PWSTR PathCopy(PWSTR Dest, PWSTR Arg, PWSTR ArgEnd, BOOLEAN WriteDest, WCHAR Replacement)
+static PWSTR PathCopy(PWSTR Dest, PWSTR Arg, PWSTR ArgEnd, BOOLEAN WriteDest, WCHAR Replacement,
+    PULONG PTrailingBackslash)
 {
+    ULONG TrailingBackslash = *PTrailingBackslash;
+
     if (0 != Replacement)
     {
         for (PWSTR P = Arg, EndP = (0 != ArgEnd ? ArgEnd : (PWSTR)(UINT_PTR)~0); EndP > P && *P; P++)
@@ -81,12 +84,14 @@ static PWSTR PathCopy(PWSTR Dest, PWSTR Arg, PWSTR ArgEnd, BOOLEAN WriteDest, WC
                 if (WriteDest)
                     *Dest = Replacement;
                 Dest++;
+                TrailingBackslash = L'\\' == Replacement ? TrailingBackslash + 2 : 0;
             }
             else if (L'"' != *P)
             {
                 if (WriteDest)
                     *Dest = *P;
                 Dest++;
+                TrailingBackslash = 0;
             }
     }
     else
@@ -97,9 +102,12 @@ static PWSTR PathCopy(PWSTR Dest, PWSTR Arg, PWSTR ArgEnd, BOOLEAN WriteDest, WC
                 if (WriteDest)
                     *Dest = *P;
                 Dest++;
+                TrailingBackslash = L'\\' == *P ? TrailingBackslash + 1 : 0;
             }
     }
 
+    *PTrailingBackslash = TrailingBackslash;
+
     return Dest;
 }
 
@@ -110,9 +118,10 @@ static inline BOOLEAN PatternEnd(WCHAR C)
         (L'A' <= C && C <= 'Z');
 }
 
-PWSTR PathTransform(PWSTR Dest, PWSTR Arg, PWSTR Pattern)
+PWSTR PathTransform(PWSTR Dest, PWSTR Arg, PWSTR Pattern, BOOLEAN Quote)
 {
     BOOLEAN WriteDest = 0 != Dest;
+    ULONG TrailingBackslash = 0;
     WCHAR Replacement;
     PWSTR Components[26][2];
     PWSTR Remainder = Arg;
@@ -120,14 +129,17 @@ PWSTR PathTransform(PWSTR Dest, PWSTR Arg, PWSTR Pattern)
     PWSTR P;
 
     if (0 == Pattern)
-        return PathCopy(Dest, Arg, 0, WriteDest, 0);
+    {
+        Dest = PathCopy(Dest, Arg, 0, WriteDest, 0, &TrailingBackslash);
+        goto exit;
+    }
 
     for (ULONG I = 0; 26 > I; I++)
         Components[I][0] = 0;
 
     Replacement = *Pattern++;
     if (PatternEnd(Replacement))
-        return Dest;
+        goto exit;
 
     while (!PatternEnd(*Pattern))
     {
@@ -163,20 +175,29 @@ PWSTR PathTransform(PWSTR Dest, PWSTR Arg, PWSTR Pattern)
                 }
             }
 
-            Dest = PathCopy(Dest, Components[I][0], Components[I][1], WriteDest, Replacement);
+            Dest = PathCopy(Dest, Components[I][0], Components[I][1], WriteDest, Replacement, &TrailingBackslash);
         }
         else
         if (L'_' == *Pattern)
-            Dest = PathCopy(Dest, Remainder, 0, WriteDest, Replacement);
+            Dest = PathCopy(Dest, Remainder, 0, WriteDest, Replacement, &TrailingBackslash);
         else
         {
             if (WriteDest)
                 *Dest = *Pattern;
             Dest++;
+            TrailingBackslash = L'\\' == *Pattern ? TrailingBackslash + 1 : 0;
         }
 
         Pattern++;
     }
 
+exit:
+    if (Quote && (TrailingBackslash & 1))
+    {
+        if (WriteDest)
+            *Dest = L'\\';
+        Dest++;
+    }
+
     return Dest;
 }

tst/winfsp-tests/launcher-ptrans-test.c

@@ -93,20 +93,153 @@ static void launcher_ptrans_test(void)
         ULONG Length;
         PWSTR Dest;
 
-        Length = (ULONG)(UINT_PTR)PathTransform(0, ipaths[2 * i + 0], ipaths[2 * i + 1]);
+        Length = (ULONG)(UINT_PTR)PathTransform(0, ipaths[2 * i + 0], ipaths[2 * i + 1], FALSE);
         ASSERT(Length == wcslen(opaths[i]) * sizeof(WCHAR));
 
-        Dest = PathTransform(Buf, ipaths[2 * i + 0], ipaths[2 * i + 1]);
+        Dest = PathTransform(Buf, ipaths[2 * i + 0], ipaths[2 * i + 1], FALSE);
         *Dest = L'\0';
         ASSERT(Dest == Buf + wcslen(opaths[i]));
         ASSERT(0 == wcscmp(Buf, opaths[i]));
     }
 }
 
+static void launcher_ptrans_quote_test(void)
+{
+    PWSTR ipaths[] =
+    {
+        L"", 0,
+        L"foo", 0,
+        L"foo\\", 0,
+        L"foo\\\\", 0,
+        L"foo\\\\\\", 0,
+        L"\\foo\\bar", 0,
+        L"foo bar\\", 0,
+        L"foo\tbar\\", 0,
+        L"foo\\\"", 0,
+        L"foo\\\"\\", 0,
+        L"foo\\\"bar", 0,
+        L"\\foo\\bar\\", L"/b",
+        L"\\foo\\bar\\", L"/_",
+        L"\\foo\\bar\" baz\\qux\\", L"/b:_",
+        L"\\foo", L"/a\\b",
+        L"\\foo\\\"", L"/a\\b",
+        L"\\foo\\bar", L"/a\\b",
+        L"\\foo", L"/a\\\\b",
+        L"", L"/\\",
+        L"\\foo\\bar\\", L"\\_",
+        L"\\foo\\bar\\\\", L"\\_",
+        L"\\", L"\\_\\b",
+        L"foo\\\\\\\\", 0,
+        L"\\", 0,
+        L"\"\"", 0,
+        L"\\foo", L"",
+        L"\\foo", L"/",
+        L"\\foo", L"/a\\b\\c",
+        L"\\foo\\bar", L"/a\\b\\c",
+        L"\\foo", L"/a\\\\\\b",
+        L"\\foo", L"/a\\:b",
+        L"\\foo", L"/a\\_",
+        L"\\foo\\\"", L"/a\\_",
+        L"\\foo\\bar\\", L"/a\\_",
+        L"\\", L"/\\_",
+        L"\"\"", L"/\\_",
+        L"\\", L"\\\\_",
+        L"\\", L"\\_\\_",
+        L"\\foo", L"/a\\z",
+        L"\\foo\\\"\\bar", L"/a\\b",
+        L"\\foo", L"/a\\1",
+        L"\\foo", L"/a\\U",
+    };
+    /* Unquoted output, quoted output, parsed argument. */
+    PWSTR opaths[][3] =
+    {
+        { L"", L"", L"" },
+        { L"foo", L"foo", L"foo" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\\\\\", L"foo\\\\\\\\", L"foo\\\\" },
+        { L"\\foo\\bar", L"\\foo\\bar", L"\\foo\\bar" },
+        { L"foo bar\\", L"foo bar\\\\", L"foo bar\\" },
+        { L"foo\tbar\\", L"foo\tbar\\\\", L"foo\tbar\\" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\bar", L"foo\\bar", L"foo\\bar" },
+        { L"bar", L"bar", L"bar" },
+        { L"/foo/bar/", L"/foo/bar/", L"/foo/bar/" },
+        { L"bar baz:qux/", L"bar baz:qux/", L"bar baz:qux/" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\bar", L"foo\\bar", L"foo\\bar" },
+        { L"foo\\\\", L"foo\\\\", L"foo\\" },
+        { L"\\", L"\\\\", L"\\" },
+        { L"\\\\foo\\\\bar\\\\", L"\\\\foo\\\\bar\\\\", L"\\\\foo\\\\bar\\" },
+        { L"\\\\foo\\\\bar\\\\\\\\", L"\\\\foo\\\\bar\\\\\\\\", L"\\\\foo\\\\bar\\\\" },
+        { L"\\\\\\", L"\\\\\\\\", L"\\\\" },
+        { L"foo\\\\\\\\", L"foo\\\\\\\\", L"foo\\\\" },
+        { L"\\", L"\\\\", L"\\" },
+        { L"", L"", L"" },
+        { L"", L"", L"" },
+        { L"", L"", L"" },
+        { L"foo\\\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\bar\\", L"foo\\bar\\\\", L"foo\\bar\\" },
+        { L"foo\\\\\\", L"foo\\\\\\\\", L"foo\\\\" },
+        { L"foo\\:", L"foo\\:", L"foo\\:" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\bar/", L"foo\\bar/", L"foo\\bar/" },
+        { L"\\/", L"\\/", L"\\/" },
+        { L"\\", L"\\\\", L"\\" },
+        { L"\\\\\\", L"\\\\\\\\", L"\\\\" },
+        { L"\\\\\\\\\\", L"\\\\\\\\\\\\", L"\\\\\\" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+        { L"foo\\", L"foo\\\\", L"foo\\" },
+    };
+
+    for (size_t i = 0; sizeof ipaths / (sizeof ipaths[0] * 2) > i; i++)
+        for (BOOLEAN Quote = FALSE; TRUE >= Quote; Quote++)
+        {
+            WCHAR Buf[1024];
+            ULONG Length;
+            PWSTR Dest;
+
+            Length = (ULONG)(UINT_PTR)PathTransform(0, ipaths[2 * i + 0], ipaths[2 * i + 1], Quote);
+            ASSERT(Length == wcslen(opaths[i][Quote]) * sizeof(WCHAR));
+
+            ASSERT(Length < sizeof Buf);
+            Buf[Length / sizeof(WCHAR)] = L'!';
+            Dest = PathTransform(Buf, ipaths[2 * i + 0], ipaths[2 * i + 1], Quote);
+            ASSERT(L'!' == *Dest);
+            *Dest = L'\0';
+            ASSERT(Dest == Buf + wcslen(opaths[i][Quote]));
+            ASSERT(0 == wcscmp(Buf, opaths[i][Quote]));
+
+            if (Quote)
+            {
+                WCHAR CommandLine[2048];
+                PWSTR *Argv;
+                int Argc;
+
+                lstrcpyW(CommandLine, L"test.exe \"");
+                lstrcatW(CommandLine, Buf);
+                lstrcatW(CommandLine, L"\" sentinel");
+                Argv = CommandLineToArgvW(CommandLine, &Argc);
+                ASSERT(0 != Argv);
+                ASSERT(3 == Argc);
+                ASSERT(0 == wcscmp(Argv[0], L"test.exe"));
+                ASSERT(0 == wcscmp(Argv[1], opaths[i][2]));
+                ASSERT(0 == wcscmp(Argv[2], L"sentinel"));
+                LocalFree(Argv);
+            }
+        }
+}
+
 void launcher_ptrans_tests(void)
 {
     if (OptExternal)
         return;
 
     TEST_OPT(launcher_ptrans_test);
+    TEST_OPT(launcher_ptrans_quote_test);
 }