Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52506 Signed-off-by: Alex Henrie alexhenrie24@gmail.com
-- v9: shell32: Sanitize Program Manager icon and group names. shell32: Introduce combine_path helper for DDE. shell32: Use standard C functions for memory allocation in dde.c. shell32: Move strndupW to dde.c.
From: Alex Henrie alexhenrie24@gmail.com
--- dlls/shell32/dde.c | 13 +++++++++++++ dlls/shell32/shell32_main.h | 13 ------------- 2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/dlls/shell32/dde.c b/dlls/shell32/dde.c index 5ab63136092..cd956def797 100644 --- a/dlls/shell32/dde.c +++ b/dlls/shell32/dde.c @@ -54,6 +54,19 @@ static const char *debugstr_hsz( HSZ hsz ) return debugstr_w( buffer ); }
+static WCHAR *strndupW(const WCHAR *src, DWORD len) +{ + WCHAR *dest; + if (!src) return NULL; + dest = heap_alloc((len + 1) * sizeof(*dest)); + if (dest) + { + memcpy(dest, src, len * sizeof(WCHAR)); + dest[len] = '\0'; + } + return dest; +} + static inline BOOL Dde_OnConnect(HSZ hszTopic, HSZ hszService) { if ((hszTopic == hszProgmanTopic) && (hszService == hszProgmanService)) diff --git a/dlls/shell32/shell32_main.h b/dlls/shell32/shell32_main.h index 7bb26e46a6e..33b85ee2726 100644 --- a/dlls/shell32/shell32_main.h +++ b/dlls/shell32/shell32_main.h @@ -241,19 +241,6 @@ static inline WCHAR *strdupW(const WCHAR *src) return dest; }
-static inline WCHAR *strndupW(const WCHAR *src, DWORD len) -{ - WCHAR *dest; - if (!src) return NULL; - dest = heap_alloc((len + 1) * sizeof(*dest)); - if (dest) - { - memcpy(dest, src, len * sizeof(WCHAR)); - dest[len] = '\0'; - } - return dest; -} - static inline WCHAR *strdupAtoW(const char *str) { WCHAR *ret;
From: Alex Henrie alexhenrie24@gmail.com
--- dlls/shell32/dde.c | 60 +++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/dlls/shell32/dde.c b/dlls/shell32/dde.c index cd956def797..39e7de9c2c9 100644 --- a/dlls/shell32/dde.c +++ b/dlls/shell32/dde.c @@ -58,7 +58,7 @@ static WCHAR *strndupW(const WCHAR *src, DWORD len) { WCHAR *dest; if (!src) return NULL; - dest = heap_alloc((len + 1) * sizeof(*dest)); + dest = malloc((len + 1) * sizeof(*dest)); if (dest) { memcpy(dest, src, len * sizeof(WCHAR)); @@ -100,7 +100,7 @@ static WCHAR *get_programs_path(const WCHAR *name) SHGetKnownFolderPath(&FOLDERID_Programs, 0, NULL, &programs);
len = lstrlenW(programs) + 1 + lstrlenW(name); - path = heap_alloc((len + 1) * sizeof(*path)); + path = malloc((len + 1) * sizeof(*path)); lstrcpyW(path, programs); lstrcatW(path, L"/"); lstrcatW(path, name); @@ -119,7 +119,7 @@ static inline HDDEDATA Dde_OnRequest(UINT uFmt, HCONV hconv, HSZ hszTopic, WIN32_FIND_DATAW finddata; HANDLE hfind; int len = 1; - WCHAR *groups_data = heap_alloc(sizeof(WCHAR)); + WCHAR *groups_data = malloc(sizeof(WCHAR)); char *groups_dataA; HDDEDATA ret;
@@ -134,7 +134,7 @@ static inline HDDEDATA Dde_OnRequest(UINT uFmt, HCONV hconv, HSZ hszTopic, wcscmp(finddata.cFileName, L".") && wcscmp(finddata.cFileName, L"..")) { len += lstrlenW(finddata.cFileName) + 2; - groups_data = heap_realloc(groups_data, len * sizeof(WCHAR)); + groups_data = realloc(groups_data, len * sizeof(WCHAR)); lstrcatW(groups_data, finddata.cFileName); lstrcatW(groups_data, L"\r\n"); } @@ -143,13 +143,13 @@ static inline HDDEDATA Dde_OnRequest(UINT uFmt, HCONV hconv, HSZ hszTopic, }
len = WideCharToMultiByte(CP_ACP, 0, groups_data, -1, NULL, 0, NULL, NULL); - groups_dataA = heap_alloc(len * sizeof(WCHAR)); + groups_dataA = malloc(len * sizeof(WCHAR)); WideCharToMultiByte(CP_ACP, 0, groups_data, -1, groups_dataA, len, NULL, NULL); ret = DdeCreateDataHandle(dwDDEInst, (BYTE *)groups_dataA, len, 0, hszGroups, uFmt, 0);
- heap_free(groups_dataA); - heap_free(groups_data); - heap_free(programs); + free(groups_dataA); + free(groups_data); + free(programs); return ret; } else if (hszTopic == hszProgmanTopic && hszItem == hszProgmanService && uFmt == CF_TEXT) @@ -180,7 +180,7 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv) CreateDirectoryW(path, NULL); ShellExecuteW(NULL, NULL, path, NULL, NULL, SW_SHOWNORMAL);
- heap_free(last_group); + free(last_group); last_group = path; } else if (!wcsicmp(command, L"DeleteGroup")) @@ -193,7 +193,7 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
path = get_programs_path(argv[0]);
- path2 = heap_alloc((lstrlenW(path) + 2) * sizeof(*path)); + path2 = malloc((lstrlenW(path) + 2) * sizeof(*path)); lstrcpyW(path2, path); path2[lstrlenW(path) + 1] = 0;
@@ -203,8 +203,8 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
ret = SHFileOperationW(&shfos);
- heap_free(path2); - heap_free(path); + free(path2); + free(path);
if (ret || shfos.fAnyOperationsAborted) return DDE_FNOTPROCESSED; } @@ -220,7 +220,7 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
ShellExecuteW(NULL, NULL, path, NULL, NULL, SW_SHOWNORMAL);
- heap_free(last_group); + free(last_group); last_group = path; } else if (!wcsicmp(command, L"AddItem")) @@ -242,10 +242,10 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv) IShellLinkW_Release(link); return DDE_FNOTPROCESSED; } - path = heap_alloc(len * sizeof(WCHAR)); + path = malloc(len * sizeof(WCHAR)); SearchPathW(NULL, argv[0], L".exe", len, path, NULL); IShellLinkW_SetPath(link, path); - heap_free(path); + free(path);
if (argc >= 2) IShellLinkW_SetDescription(link, argv[1]); if (argc >= 4) IShellLinkW_SetIconLocation(link, argv[2], wcstol(argv[3], NULL, 10)); @@ -266,19 +266,19 @@ 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)); + name = malloc(len * sizeof(*name)); swprintf( name, len, L"%s/%s.lnk", last_group, argv[1] ); } else { const WCHAR *filename = PathFindFileNameW(argv[0]); len = PathFindExtensionW(filename) - filename; - name = heap_alloc((lstrlenW(last_group) + 1 + len + 5) * sizeof(*name)); + name = malloc((lstrlenW(last_group) + 1 + len + 5) * sizeof(*name)); swprintf( name, lstrlenW(last_group) + 1 + len + 5, L"%s/%.*s.lnk", last_group, len, filename ); } hres = IPersistFile_Save(file, name, TRUE);
- heap_free(name); + free(name); IPersistFile_Release(file); IShellLinkW_Release(link);
@@ -292,12 +292,12 @@ 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)); + name = malloc(len * sizeof(*name)); swprintf( name, len, L"%s/%s.lnk", last_group, argv[0]);
ret = DeleteFileW(name);
- heap_free(name); + free(name);
if (!ret) return DDE_FNOTPROCESSED; } @@ -326,7 +326,7 @@ static DWORD parse_dde_command(HSZ hszTopic, WCHAR *command) while (*command == '[') { argc = 0; - argv = heap_alloc(sizeof(*argv)); + argv = malloc(sizeof(*argv));
command++; while (*command == ' ') command++; @@ -355,7 +355,7 @@ static DWORD parse_dde_command(HSZ hszTopic, WCHAR *command) }
argc++; - argv = heap_realloc(argv, argc * sizeof(*argv)); + argv = realloc(argv, argc * sizeof(*argv)); argv[argc-1] = strndupW(command, p - command);
command = p; @@ -381,9 +381,9 @@ static DWORD parse_dde_command(HSZ hszTopic, WCHAR *command) ret = DDE_FNOTPROCESSED; }
- heap_free(opcode); - for (i = 0; i < argc; i++) heap_free(argv[i]); - heap_free(argv); + free(opcode); + for (i = 0; i < argc; i++) free(argv[i]); + free(argv);
if (ret == DDE_FNOTPROCESSED) break; } @@ -392,9 +392,9 @@ static DWORD parse_dde_command(HSZ hszTopic, WCHAR *command)
error: ERR("failed to parse command %s\n", debugstr_w(original)); - heap_free(opcode); - for (i = 0; i < argc; i++) heap_free(argv[i]); - heap_free(argv); + free(opcode); + for (i = 0; i < argc; i++) free(argv[i]); + free(argv); return DDE_FNOTPROCESSED; }
@@ -406,14 +406,14 @@ static DWORD Dde_OnExecute(HCONV hconv, HSZ hszTopic, HDDEDATA hdata)
len = DdeGetData(hdata, NULL, 0, 0); if (!len) return DDE_FNOTPROCESSED; - command = heap_alloc(len); + command = malloc(len); DdeGetData(hdata, (BYTE *)command, len, 0);
TRACE("conv=%p topic=%s data=%s\n", hconv, debugstr_hsz(hszTopic), debugstr_w(command));
ret = parse_dde_command(hszTopic, command);
- heap_free(command); + free(command); return ret; }
From: Alex Henrie alexhenrie24@gmail.com
--- dlls/shell32/dde.c | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/dlls/shell32/dde.c b/dlls/shell32/dde.c index 39e7de9c2c9..51aa8f88927 100644 --- a/dlls/shell32/dde.c +++ b/dlls/shell32/dde.c @@ -91,20 +91,30 @@ static inline BOOL Dde_OnWildConnect(HSZ hszTopic, HSZ hszService) return FALSE; }
+static WCHAR *combine_path(const WCHAR *directory, const WCHAR *name, const WCHAR *extension) +{ + WCHAR *path; + int len; + + len = wcslen(directory) + 1 + wcslen(name); + if (extension) len += wcslen(extension); + path = malloc((len + 1) * sizeof(WCHAR)); + + 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) { WCHAR *programs, *path; - int len;
SHGetKnownFolderPath(&FOLDERID_Programs, 0, NULL, &programs); - - len = lstrlenW(programs) + 1 + lstrlenW(name); - path = malloc((len + 1) * sizeof(*path)); - lstrcpyW(path, programs); - lstrcatW(path, L"/"); - lstrcatW(path, name); - + path = combine_path(programs, name, NULL); CoTaskMemFree(programs);
return path; @@ -265,16 +275,15 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv) } if (argc >= 2) { - len = lstrlenW(last_group) + 1 + lstrlenW(argv[1]) + 5; - name = malloc(len * sizeof(*name)); - swprintf( name, len, L"%s/%s.lnk", last_group, argv[1] ); + name = combine_path(last_group, argv[1], L".lnk"); } else { - const WCHAR *filename = PathFindFileNameW(argv[0]); - len = PathFindExtensionW(filename) - filename; - name = malloc((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 = wcsdup(PathFindFileNameW(argv[0])); + WCHAR *ext = PathFindExtensionW(filename); + *ext = '\0'; + name = combine_path(last_group, filename, L".lnk"); + free(filename); } hres = IPersistFile_Save(file, name, TRUE);
@@ -291,12 +300,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 = malloc(len * sizeof(*name)); - swprintf( name, len, L"%s/%s.lnk", last_group, argv[0]); - + name = combine_path(last_group, argv[0], L".lnk"); ret = DeleteFileW(name); - free(name);
if (!ret) return DDE_FNOTPROCESSED;
From: Alex Henrie alexhenrie24@gmail.com
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52506 --- dlls/shell32/dde.c | 40 ++++++++++++++++++++++---------- dlls/shell32/tests/progman_dde.c | 3 +-- 2 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/dlls/shell32/dde.c b/dlls/shell32/dde.c index 51aa8f88927..24482d2c33f 100644 --- a/dlls/shell32/dde.c +++ b/dlls/shell32/dde.c @@ -91,16 +91,32 @@ static inline BOOL Dde_OnWildConnect(HSZ hszTopic, HSZ hszService) return FALSE; }
-static WCHAR *combine_path(const WCHAR *directory, const WCHAR *name, const WCHAR *extension) +static WCHAR *combine_path(const WCHAR *directory, const WCHAR *name, const WCHAR *extension, BOOL sanitize) { WCHAR *path; - int len; + int len, i;
len = wcslen(directory) + 1 + wcslen(name); if (extension) len += wcslen(extension); path = malloc((len + 1) * sizeof(WCHAR));
- PathCombineW(path, directory, name); + 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); + free(sanitized_name); + } + else + { + PathCombineW(path, directory, name); + }
if (extension) wcscat(path, extension); @@ -109,12 +125,12 @@ static WCHAR *combine_path(const WCHAR *directory, const WCHAR *name, const WCHA }
/* 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;
SHGetKnownFolderPath(&FOLDERID_Programs, 0, NULL, &programs); - path = combine_path(programs, name, NULL); + path = combine_path(programs, name, NULL, sanitize); CoTaskMemFree(programs);
return path; @@ -134,7 +150,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) { @@ -185,7 +201,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); @@ -201,7 +217,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 = malloc((lstrlenW(path) + 2) * sizeof(*path)); lstrcpyW(path2, path); @@ -226,7 +242,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);
@@ -275,14 +291,14 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv) } if (argc >= 2) { - name = combine_path(last_group, argv[1], L".lnk"); + name = combine_path(last_group, argv[1], L".lnk", TRUE); } else { WCHAR *filename = wcsdup(PathFindFileNameW(argv[0])); WCHAR *ext = PathFindExtensionW(filename); *ext = '\0'; - name = combine_path(last_group, filename, L".lnk"); + name = combine_path(last_group, filename, L".lnk", TRUE); free(filename); } hres = IPersistFile_Save(file, name, TRUE); @@ -300,7 +316,7 @@ static DWORD PROGMAN_OnExecute(WCHAR *command, int argc, WCHAR **argv)
if (argc < 1) return DDE_FNOTPROCESSED;
- name = combine_path(last_group, argv[0], L".lnk"); + name = combine_path(last_group, argv[0], L".lnk", FALSE); ret = DeleteFileW(name); free(name);
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);
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=124994
Your paranoid android.
=== debian11 (32 bit report) ===
ddraw: ddraw7.c:17858: Test succeeded inside todo block: Got unexpected caps 0x3046737c. ddraw7.c:17884: Test failed: Got unexpected caps 0x3046737c.
=== debian11 (32 bit ar:MA report) ===
shell32: progman_dde.c:259: Test failed: expected DMLERR_NO_ERROR, got 16393 progman_dde.c:260: Test failed: directory should not exist
=== debian11 (32 bit zh:CN report) ===
shell32: autocomplete.c:614: Test failed: Expected (null), got L"ab" autocomplete.c:637: Test failed: Expected (null), got L"www.ax"
=== debian11 (build log) ===
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24740. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24740. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24740.
On Thu Oct 13 17:15:26 2022 +0000, Huw Davies wrote:
I don't think adding another use case of the non-standard `strndupW()` makes things less ugly. Go back to using `wcsdup()` - it doesn't matter that we over-allocate a little as this is only a short-lived string.
Okay, done.
This merge request was approved by Huw Davies.
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Huw Davies (＠huw)
-
Marvin