[PATCH 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>

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

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

[Alex Henrie]

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

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>
---
 dlls/shell32/dde.c               |  75 +++++---
 dlls/shell32/tests/progman_dde.c | 318 ++++++++++++++++++++++++++++++-
 2 files changed, 368 insertions(+), 25 deletions(-)

diff --git a/dlls/shell32/dde.c b/dlls/shell32/dde.c
index 5ab63136092..5138843f259 100644
--- a/dlls/shell32/dde.c
+++ b/dlls/shell32/dde.c
@@ -78,19 +78,49 @@ static inline BOOL Dde_OnWildConnect(HSZ hszTopic, HSZ hszService)
     return FALSE;
 }
 
+static WCHAR *sanitize_and_combine(const WCHAR *directory, WCHAR *name, BOOL icon)
+{
+    WCHAR *path;
+    int len, i;
+
+    len = lstrlenW(directory) + 1 + lstrlenW(name);
+    if (icon) len += 4;
+    path = heap_alloc((len + 1) * sizeof(WCHAR));
+
+    for (i = 0; i < lstrlenW(name); i++)
+    {
+        if (name[i] < ' ' || StrChrW(L"*/:<>?\\|", name[i]))
+            name[i] = '_';
+    }
+
+    PathCombineW(path, directory, name);
+
+    if (icon)
+        lstrcatW(path, L".lnk");
+
+    return path;
+}
+
 /* Returned string must be freed by caller */
-static WCHAR *get_programs_path(const WCHAR *name)
+static WCHAR *get_programs_path(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);
+    if (sanitize)
+    {
+        path = sanitize_and_combine(programs, name, FALSE);
+    }
+    else
+    {
+        len = lstrlenW(programs) + 1 + lstrlenW(name);
+        path = heap_alloc((len + 1) * sizeof(WCHAR));
+        lstrcpyW(path, programs);
+        lstrcatW(path, L"/");
+        lstrcatW(path, name);
+    }
 
     CoTaskMemFree(programs);
 
@@ -109,9 +139,10 @@ static inline HDDEDATA Dde_OnRequest(UINT uFmt, HCONV hconv, HSZ hszTopic,
         WCHAR *groups_data = heap_alloc(sizeof(WCHAR));
         char *groups_dataA;
         HDDEDATA ret;
+        static WCHAR star[] = L"*";
 
         groups_data[0] = 0;
-        programs = get_programs_path(L"*");
+        programs = get_programs_path(star, FALSE);
         hfind = FindFirstFileW(programs, &finddata);
         if (hfind)
         {
@@ -162,10 +193,10 @@ 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);
+        if (CreateDirectoryW(path, NULL))
+            ShellExecuteW(NULL, NULL, path, NULL, NULL, SW_SHOWNORMAL);
 
         heap_free(last_group);
         last_group = path;
@@ -178,7 +209,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 +234,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);
 
@@ -250,26 +281,21 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
             IShellLinkW_Release(link);
             return DDE_FNOTPROCESSED;
         }
-        if (argc >= 2)
+        if (argc >= 2 && argv[1][0])
         {
-            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 = sanitize_and_combine(last_group, argv[1], 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 = sanitize_and_combine(last_group, filename, TRUE);
         }
-        hres = IPersistFile_Save(file, name, TRUE);
+        IPersistFile_Save(file, name, TRUE);
 
         heap_free(name);
         IPersistFile_Release(file);
         IShellLinkW_Release(link);
-
-        if (FAILED(hres)) return DDE_FNOTPROCESSED;
     }
     else if (!wcsicmp(command, L"DeleteItem") || !wcsicmp(command, L"ReplaceItem"))
     {
@@ -280,7 +306,8 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
 
         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]);
+        PathCombineW(name, last_group, argv[0]);
+        lstrcatW(name, L".lnk");
 
         ret = DeleteFileW(name);
 
diff --git a/dlls/shell32/tests/progman_dde.c b/dlls/shell32/tests/progman_dde.c
index 5d532f9222f..66b9ed0c87a 100644
--- a/dlls/shell32/tests/progman_dde.c
+++ b/dlls/shell32/tests/progman_dde.c
@@ -23,7 +23,7 @@
  *   functionality
  * - Todo: Handle CommonGroupFlag
  *         Better AddItem Tests (Lots of parameters to test)
- *         Tests for Invalid Characters in Names / Invalid Parameters
+ *         Tests for invalid parameters
  */
 
 #include <stdio.h>
@@ -425,6 +425,321 @@ static void test_request_groups(DWORD instance, HCONV hconv)
     FindClose(hfind);
 }
 
+static BOOL is_unsanitary(char c)
+{
+    return (c > 0 && c < ' ') || strchr("*/:<>?\\|", c) != NULL;
+}
+
+static void sanitize_name(const char *original_name, char *sanitized_name, BOOL group)
+{
+    BOOL at_end = TRUE;
+    int i;
+
+    i = strlen(original_name);
+    sanitized_name[i] = 0;
+
+    while (--i >= 0)
+    {
+        if (is_unsanitary(original_name[i]))
+        {
+            /* replaced in all positions */
+            sanitized_name[i] = '_';
+            at_end = FALSE;
+        }
+        else if (original_name[i] == '.' || (original_name[i] == ' ' && group))
+        {
+            /* left alone if in the middle of the string, dropped if at the end of the string */
+            sanitized_name[i] = at_end ? '\0' : original_name[i];
+        }
+        else
+        {
+            /* left alone in all positions */
+            sanitized_name[i] = original_name[i];
+            at_end = FALSE;
+        }
+    }
+}
+
+static void test_name_sanitization(DWORD instance, HCONV hConv)
+{
+    static const char test_chars[] = "\x01\x1F !#$%&'*+,-./:;<=>?@[\\]^`{|}~\x7F\x80\xFF";
+    char original_name[16], sanitized_icon_name[16], sanitized_group_name[16];
+    char buf[64];
+    UINT error;
+    int i;
+    char c;
+
+    if (0) /* the directory isn't deleted on windows < 7 */
+    {
+        error = dde_execute(instance, hConv, "[CreateGroup(\" \")]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        ok(check_exists(" "), "directory not created\n");
+        ok(!check_window_exists(" "), "window should not exist\n");
+
+        error = dde_execute(instance, hConv, "[DeleteGroup(\" \")]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        ok(!check_exists(" "), "directory should not exist\n");
+    }
+
+    if (GetUserDefaultUILanguage() == MAKELANGID(LANG_ENGLISH, SUBLANG_DEFAULT))
+    {
+        error = dde_execute(instance, hConv, "[CreateGroup(\"\")]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        todo_wine ok(check_window_exists("Programs") || broken(TRUE) /* 2003 */, "window not created\n");
+
+        error = dde_execute(instance, hConv, "[CreateGroup(\".\")]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        ok(!check_window_exists("Programs"), "window should not exist\n");
+
+        error = dde_execute(instance, hConv, "[CreateGroup(\"..\")]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        todo_wine ok(check_window_exists("Start Menu") || broken(TRUE) /* 2003 */, "window not created\n");
+
+        error = dde_execute(instance, hConv, "[CreateGroup(\"...\")]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        todo_wine ok(check_window_exists("Programs") || broken(TRUE) /* XP */, "window not created\n");
+
+        error = dde_execute(instance, hConv, "[CreateGroup(\"....\")]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        ok(!check_window_exists("Programs"), "window should not exist\n");
+    }
+    else
+    {
+        skip("Directory names are probably not in English\n");
+    }
+
+    if (0) /* these calls will actually delete the start menu */
+    {
+        error = dde_execute(instance, hConv, "[DeleteGroup(\"\")]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        ok(!check_exists("../Programs"), "directory should not exist\n");
+
+        error = dde_execute(instance, hConv, "[DeleteGroup(\"..\")]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        ok(!check_exists("../../Start Menu"), "directory should not exist\n");
+    }
+
+    error = dde_execute(instance, hConv, "[CreateGroup(Group3)]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(check_exists("Group3"), "directory not created\n");
+    ok(check_window_exists("Group3"), "window not created\n");
+
+    error = dde_execute(instance, hConv, "[AddItem(notepad,\"\")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(check_exists("Group3/notepad.lnk"), "link not created\n");
+
+    error = dde_execute(instance, hConv, "[DeleteItem(notepad)]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(!check_exists("Group3/notepad.lnk"), "link should not exist\n");
+
+    error = dde_execute(instance, hConv, "[AddItem(notepad,\" \")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(check_exists("Group3/ .lnk"), "link not created\n");
+
+    error = dde_execute(instance, hConv, "[DeleteItem(\" \")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(!check_exists("Group3/ .lnk"), "link should not exist\n");
+
+    error = dde_execute(instance, hConv, "[AddItem(notepad,\".\")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    todo_wine ok(check_exists("Group3.lnk"), "link not created\n");
+
+    error = dde_execute(instance, hConv, "[DeleteItem(\".\")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(!check_exists("Group3.lnk"), "link should not exist\n");
+
+    error = dde_execute(instance, hConv, "[AddItem(notepad,\"..\")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(check_exists("../Programs.lnk"), "link not created\n");
+
+    error = dde_execute(instance, hConv, "[DeleteItem(\"..\")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(!check_exists("../Programs.lnk"), "link should not exist\n");
+
+    error = dde_execute(instance, hConv, "[AddItem(notepad,\"...\")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    todo_wine ok(check_exists("Group3/.lnk") || broken(TRUE) /* XP */, "link not created\n");
+
+    error = dde_execute(instance, hConv, "[DeleteItem(\"...\")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(!check_exists("Group3/.lnk"), "link should not exist\n");
+
+    error = dde_execute(instance, hConv, "[AddItem(notepad,\"....\")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    todo_wine ok(check_exists("Group3/.lnk") || broken(TRUE) /* XP */, "link not created\n");
+
+    error = dde_execute(instance, hConv, "[DeleteItem(\"....\")]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(!check_exists("Group3/.lnk"), "link should not exist\n");
+
+    /* Test sanitary group name with unsanitary icon names */
+
+    for (i = 0; i < sizeof(test_chars) - 1; i++)
+    {
+        c = test_chars[i];
+        winetest_push_context("char %d '%c'", c, c);
+
+        sprintf(original_name, "%03d_%c_.%c", c, c, c);
+        sanitize_name(original_name, sanitized_icon_name, FALSE);
+
+        sprintf(buf, "[AddItem(notepad,\"Notepad%s\")]", original_name);
+        error = dde_execute(instance, hConv, buf);
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        sprintf(buf, "Group3/Notepad%s.lnk", sanitized_icon_name);
+        todo_wine_if(c == '.') ok(check_exists(buf) || broken(c == '.') /* XP */, "link not created\n");
+        if (!check_exists(buf))
+        {
+            winetest_pop_context();
+            continue;
+        }
+
+        if (is_unsanitary(c))
+        {
+            sprintf(buf, "[ReplaceItem(\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NOTPROCESSED, "expected DMLERR_NOTPROCESSED, got %#x\n", error);
+            sprintf(buf, "Group3/Notepad%s.lnk", sanitized_icon_name);
+            ok(check_exists(buf), "link should still exist\n");
+
+            sprintf(buf, "[DeleteItem(\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NOTPROCESSED, "expected DMLERR_NOTPROCESSED, got %#x\n", error);
+            sprintf(buf, "Group3/Notepad%s.lnk", sanitized_icon_name);
+            ok(check_exists(buf), "link should still exist\n");
+        }
+        else
+        {
+            sprintf(buf, "[ReplaceItem(\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+            sprintf(buf, "Group3/Notepad%s.lnk", sanitized_icon_name);
+            ok(!check_exists(buf), "link should not exist\n");
+
+            sprintf(buf, "[AddItem(notepad,\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+            sprintf(buf, "Group3/Notepad%s.lnk", sanitized_icon_name);
+            ok(check_exists(buf), "link not created\n");
+
+            sprintf(buf, "[DeleteItem(\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+            sprintf(buf, "Group3/Notepad%s.lnk", sanitized_icon_name);
+            ok(!check_exists(buf), "link should not exist\n");
+        }
+
+        winetest_pop_context();
+    }
+
+    error = dde_execute(instance, hConv, "[DeleteGroup(Group3)]");
+    ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+    ok(!check_exists("Group3"), "directory should not exist\n");
+
+    for (i = 0; i < sizeof(test_chars) - 1; i++)
+    {
+        c = test_chars[i];
+        winetest_push_context("char %d '%c'", c, c);
+
+        sprintf(original_name, "%03d_%c_.%c", c, c, c);
+        sanitize_name(original_name, sanitized_group_name, TRUE);
+        sanitize_name(original_name, sanitized_icon_name, FALSE);
+
+        sprintf(buf, "[CreateGroup(\"Group%s\")]", original_name);
+        error = dde_execute(instance, hConv, buf);
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        sprintf(buf, "Group%s", sanitized_group_name);
+        ok(check_exists(buf), "directory not created\n");
+        ok(check_window_exists(buf) || broken(c == ' ') /* vista */, "window not created\n");
+
+        sprintf(buf, "[ShowGroup(\"Group%s\", 0)]", original_name);
+        error = dde_execute(instance, hConv, buf);
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        sprintf(buf, "Group%s", sanitized_group_name);
+        ok(check_window_exists(buf) || broken(c == ' ') /* vista */, "window not created\n");
+
+        /* Test unsanitary group name with sanitary icon name */
+
+        error = dde_execute(instance, hConv, "[AddItem(notepad,Notepad)]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        sprintf(buf, "Group%s/Notepad.lnk", sanitized_group_name);
+        if (c == ' ')
+        {
+            /* Although no error is reported, no icon is created if the group name ends in a space */
+            ok(!check_exists(buf), "link should not exist\n");
+            goto delete_group;
+        }
+        todo_wine_if(c == '.') ok(check_exists(buf), "link not created\n");
+
+        error = dde_execute(instance, hConv, "[ReplaceItem(Notepad)]");
+        todo_wine_if(c == '.') ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        sprintf(buf, "Group%s/Notepad.lnk", sanitized_group_name);
+        ok(!check_exists(buf), "link should not exist\n");
+
+        error = dde_execute(instance, hConv, "[AddItem(notepad,Notepad)]");
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        sprintf(buf, "Group%s/Notepad.lnk", sanitized_group_name);
+        todo_wine_if(c == '.') ok(check_exists(buf), "link not created\n");
+
+        error = dde_execute(instance, hConv, "[DeleteItem(Notepad)]");
+        todo_wine_if(c == '.') ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        sprintf(buf, "Group%s/Notepad.lnk", sanitized_group_name);
+        ok(!check_exists(buf), "link should not exist\n");
+
+        /* Test unsanitary group name with unsanitary icon name */
+
+        sprintf(buf, "[AddItem(notepad,\"Notepad%s\")]", original_name);
+        error = dde_execute(instance, hConv, buf);
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        sprintf(buf, "Group%s/Notepad%s.lnk", sanitized_group_name, sanitized_icon_name);
+        todo_wine_if(c == '.') ok(check_exists(buf) || broken(c == '.') /* XP */, "link not created\n");
+        if (!check_exists(buf)) goto delete_group;
+
+        if (is_unsanitary(c))
+        {
+            sprintf(buf, "[ReplaceItem(\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NOTPROCESSED, "expected DMLERR_NOTPROCESSED, got %#x\n", error);
+            sprintf(buf, "Group%s/Notepad%s.lnk", sanitized_group_name, sanitized_icon_name);
+            ok(check_exists(buf), "link should still exist\n");
+
+            sprintf(buf, "[DeleteItem(\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NOTPROCESSED, "expected DMLERR_NOTPROCESSED, got %#x\n", error);
+            sprintf(buf, "Group%s/Notepad%s.lnk", sanitized_group_name, sanitized_icon_name);
+            ok(check_exists(buf), "link should still exist\n");
+        }
+        else
+        {
+            sprintf(buf, "[ReplaceItem(\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+            sprintf(buf, "Group%s/Notepad%s.lnk", sanitized_group_name, sanitized_icon_name);
+            ok(!check_exists(buf), "link should not exist\n");
+
+            sprintf(buf, "[AddItem(notepad,\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+            sprintf(buf, "Group%s/Notepad%s.lnk", sanitized_group_name, sanitized_icon_name);
+            ok(check_exists(buf), "link not created\n");
+
+            sprintf(buf, "[DeleteItem(\"Notepad%s\")]", original_name);
+            error = dde_execute(instance, hConv, buf);
+            ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+            sprintf(buf, "Group%s/Notepad%s.lnk", sanitized_group_name, sanitized_icon_name);
+            ok(!check_exists(buf), "link should not exist\n");
+        }
+
+delete_group:
+        sprintf(buf, "[DeleteGroup(\"Group%s\")]", original_name);
+        error = dde_execute(instance, hConv, buf);
+        ok(error == DMLERR_NO_ERROR, "expected DMLERR_NO_ERROR, got %#x\n", error);
+        sprintf(buf, "Group%s", sanitized_group_name);
+        ok(!check_exists(buf), "directory should not exist\n");
+
+        winetest_pop_context();
+    }
+}
+
 START_TEST(progman_dde)
 {
     DWORD instance = 0;
@@ -479,6 +794,7 @@ START_TEST(progman_dde)
 
     /* Run Tests */
     test_progman_dde2(instance, hConv);
+    test_name_sanitization(instance, hConv);
 
     /* Cleanup & Exit */
     ret = DdeDisconnect(hConv);
-- 
GitLab

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

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

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=113968

Your paranoid android.


=== debian11 (32 bit WoW report) ===

shell32:
shelllink.c:751: Test failed: dirty (0x00000000)
shelllink.c:751: Test failed: got 0x00000001
shelllink.c:751: Test failed: Didn't expect NULL
Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x6a2e92cd).

MR23v1 - shell32: Sanitize Program Manager icon and group names

[Alexandre Julliard (＠julliard)]

Alexandre Julliard (@julliard) commented about dlls/shell32/dde.c:
> WCHAR *groups_data = heap_alloc(sizeof(WCHAR));
>          char *groups_dataA;
>          HDDEDATA ret;
> +        static WCHAR star[] = L"*";
>
> groups_data[0] = 0;
> -        programs = get_programs_path(L"*");
> +        programs = get_programs_path(star, FALSE);

It's ugly to have to pass a non-const buffer that actually won't get modified. You should rethink your helper functions.

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