[PATCH v4 0/1] MR23: shell32: Sanitize Program Manager icon and group names

[Alex Henrie (＠alexhenrie)]

All of the new tests that are marked todo_wine fail because Wine's
PathCombineW does not handle ".", "...", "....", or "foobar." correctly.
They will pass once PathCombineW is fixed.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52506
Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com>

--
  v4: shell32: Sanitize Program Manager icon and group names.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/23

[PATCH v4 1/1] shell32: Sanitize Program Manager icon and group names.

[Alex Henrie]

From: Alex Henrie <alexhenrie24(a)gmail.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52506
Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com>
---
 dlls/shell32/dde.c               | 69 ++++++++++++++++++++------------
 dlls/shell32/tests/progman_dde.c |  3 +-
 2 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/dlls/shell32/dde.c b/dlls/shell32/dde.c
index 5ab63136092..d4a9893b65d 100644
--- a/dlls/shell32/dde.c
+++ b/dlls/shell32/dde.c
@@ -78,20 +78,46 @@ static inline BOOL Dde_OnWildConnect(HSZ hszTopic, HSZ hszService)
     return FALSE;
 }
 
+static WCHAR *combine_path(const WCHAR *directory, const WCHAR *name, const WCHAR *extension, BOOL sanitize)
+{
+    WCHAR *path;
+    int len, i;
+
+    len = wcslen(directory) + 1 + wcslen(name);
+    if (extension) len += wcslen(extension);
+    path = heap_alloc((len + 1) * sizeof(WCHAR));
+
+    if (sanitize)
+    {
+        WCHAR *sanitized_name = wcsdup(name);
+
+        for (i = 0; i < wcslen(name); i++)
+        {
+            if (name[i] < ' ' || wcschr(L"*/:<>?\\|", name[i]))
+                sanitized_name[i] = '_';
+        }
+
+        PathCombineW(path, directory, sanitized_name);
+        heap_free(sanitized_name);
+    }
+    else
+    {
+        PathCombineW(path, directory, name);
+    }
+
+    if (extension)
+        wcscat(path, extension);
+
+    return path;
+}
+
 /* Returned string must be freed by caller */
-static WCHAR *get_programs_path(const WCHAR *name)
+static WCHAR *get_programs_path(const WCHAR *name, BOOL sanitize)
 {
     WCHAR *programs, *path;
-    int len;
 
     SHGetKnownFolderPath(&FOLDERID_Programs, 0, NULL, &programs);
-
-    len = lstrlenW(programs) + 1 + lstrlenW(name);
-    path = heap_alloc((len + 1) * sizeof(*path));
-    lstrcpyW(path, programs);
-    lstrcatW(path, L"/");
-    lstrcatW(path, name);
-
+    path = combine_path(programs, name, NULL, sanitize);
     CoTaskMemFree(programs);
 
     return path;
@@ -111,7 +137,7 @@ static inline HDDEDATA Dde_OnRequest(UINT uFmt, HCONV hconv, HSZ hszTopic,
         HDDEDATA ret;
 
         groups_data[0] = 0;
-        programs = get_programs_path(L"*");
+        programs = get_programs_path(L"*", FALSE);
         hfind = FindFirstFileW(programs, &finddata);
         if (hfind)
         {
@@ -162,7 +188,7 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
 
         if (argc < 1) return DDE_FNOTPROCESSED;
 
-        path = get_programs_path(argv[0]);
+        path = get_programs_path(argv[0], TRUE);
 
         CreateDirectoryW(path, NULL);
         ShellExecuteW(NULL, NULL, path, NULL, NULL, SW_SHOWNORMAL);
@@ -178,7 +204,7 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
 
         if (argc < 1) return DDE_FNOTPROCESSED;
 
-        path = get_programs_path(argv[0]);
+        path = get_programs_path(argv[0], TRUE);
 
         path2 = heap_alloc((lstrlenW(path) + 2) * sizeof(*path));
         lstrcpyW(path2, path);
@@ -203,7 +229,7 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
          * ignore its actual value. */
         if (argc < 2) return DDE_FNOTPROCESSED;
 
-        path = get_programs_path(argv[0]);
+        path = get_programs_path(argv[0], TRUE);
 
         ShellExecuteW(NULL, NULL, path, NULL, NULL, SW_SHOWNORMAL);
 
@@ -252,16 +278,13 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
         }
         if (argc >= 2)
         {
-            len = lstrlenW(last_group) + 1 + lstrlenW(argv[1]) + 5;
-            name = heap_alloc(len * sizeof(*name));
-            swprintf( name, len, L"%s/%s.lnk", last_group, argv[1] );
+            name = combine_path(last_group, argv[1], L".lnk", TRUE);
         }
         else
         {
-            const WCHAR *filename = PathFindFileNameW(argv[0]);
-            len = PathFindExtensionW(filename) - filename;
-            name = heap_alloc((lstrlenW(last_group) + 1 + len + 5) * sizeof(*name));
-            swprintf( name, lstrlenW(last_group) + 1 + len + 5, L"%s/%.*s.lnk", last_group, len, filename );
+            WCHAR *filename = PathFindFileNameW(argv[0]);
+            *PathFindExtensionW(filename) = '\0';
+            name = combine_path(last_group, filename, L".lnk", TRUE);
         }
         hres = IPersistFile_Save(file, name, TRUE);
 
@@ -278,12 +301,8 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
 
         if (argc < 1) return DDE_FNOTPROCESSED;
 
-        len = lstrlenW(last_group) + 1 + lstrlenW(argv[0]) + 5;
-        name = heap_alloc(len * sizeof(*name));
-        swprintf( name, len, L"%s/%s.lnk", last_group, argv[0]);
-
+        name = combine_path(last_group, argv[0], L".lnk", FALSE);
         ret = DeleteFileW(name);
-
         heap_free(name);
 
         if (!ret) return DDE_FNOTPROCESSED;
diff --git a/dlls/shell32/tests/progman_dde.c b/dlls/shell32/tests/progman_dde.c
index cb1ff8ddb9f..5a7e8c912e5 100644
--- a/dlls/shell32/tests/progman_dde.c
+++ b/dlls/shell32/tests/progman_dde.c
@@ -459,8 +459,7 @@ static void test_name_sanitization(DWORD instance, HCONV hConv)
     error = dde_execute(instance, hConv, buf);
     ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
     sprintf(buf, "Group%s", sanitized_name);
-    todo_wine ok(check_exists(buf), "directory not created\n");
-    if (!check_exists(buf)) return;
+    ok(check_exists(buf), "directory not created\n");
     ok(check_window_exists(buf), "window not created\n");
 
     sprintf(buf, "[ShowGroup(\"Group%s\", 0)]", original_name);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/23