[PATCH v4 0/2] MR9882: kernelbase: Trim whitespaces from the quoted 'cmdline'
If 'name' does not have a suffix, a suffix will be appended in find_exe_file. If 'name' contains spaces, it will result in an incorrect filename being concatenated. In the log of a certain installer, I saw an entry like this: trace:process:CreateProcessInternalW app (null) cmdline L"\"C:\\Program Files\\some folder\\7za \" x -y -aos \"-oC://Program Files//target folder/\" \"C://Program Files//source.zip\"" Signed-off-by: YeshunYe <yeyeshun@uniontech.com> -- v4: kernelbase: Trim whitespaces from the quoted 'cmdline' kernel32/tests: Add tests for CreateProcessA https://gitlab.winehq.org/wine/wine/-/merge_requests/9882
From: YeshunYe <yeyeshun@uniontech.com> test for cmd with quotes and whitespaces Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- dlls/kernel32/tests/process.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 9d6dfb126e2..a77e2f4f328 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -1155,6 +1155,41 @@ static void test_CommandLine(void) cmdline = GetCommandLineW(); ok(cmdline == cmdline_backup, "Expected cached address from TEB, got %p\n", cmdline); NtCurrentTeb()->Peb->ProcessParameters->CommandLine.Buffer = cmdline_backup; + + /* Test quoted command line without file extension*/ + sprintf(buffer, "%s", selfname); + p = strrchr(buffer, '.'); + *p = 0; + + get_file_name(resfile); + sprintf(buffer2, "\" %s\" process dump \"%s\"", buffer, resfile); + ret = CreateProcessA(NULL, buffer2, NULL, NULL, FALSE, 0L, NULL, NULL, &startup, &info); + ok(!ret, "CreateProcessA unexpectedly succeeded\n"); + ok(GetLastError() == ERROR_FILE_NOT_FOUND, "Expected ERROR_FILE_NOT_FOUND, got %ld\n", GetLastError()); + + get_file_name(resfile); + sprintf(buffer2, "\"%s \" process dump \"%s\"", buffer, resfile); + ret = CreateProcessA(NULL, buffer2, NULL, NULL, FALSE, 0L, NULL, NULL, &startup, &info); + todo_wine + ok(ret, "CreateProcess (%s) failed : %ld\n", buffer, GetLastError()); + if (info.hProcess) + { + wait_child_process(&info); + release_memory(); + DeleteFileA(resfile); + } + + get_file_name(resfile); + sprintf(buffer2, "\"\t%s\" process dump \"%s\"", buffer, resfile); + ret = CreateProcessA(NULL, buffer2, NULL, NULL, FALSE, 0L, NULL, NULL, &startup, &info); + ok(!ret, "CreateProcessA unexpectedly succeeded\n"); + ok(GetLastError() == ERROR_FILE_NOT_FOUND, "Expected ERROR_FILE_NOT_FOUND, got %ld\n", GetLastError()); + + get_file_name(resfile); + sprintf(buffer2, "\"%s\t\" process dump \"%s\"", buffer, resfile); + ret = CreateProcessA(NULL, buffer2, NULL, NULL, FALSE, 0L, NULL, NULL, &startup, &info); + ok(!ret, "CreateProcessA unexpectedly succeeded\n"); + ok(GetLastError() == ERROR_FILE_NOT_FOUND, "Expected ERROR_FILE_NOT_FOUND, got %ld\n", GetLastError()); } static void test_Directory(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9882
From: YeshunYe <yeyeshun@uniontech.com> If 'name' does not have a suffix, a suffix will be appended in find_exe_file. If 'name' contains spaces, it will result in an incorrect filename being concatenated. Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- dlls/kernel32/tests/process.c | 1 - dlls/kernelbase/process.c | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index a77e2f4f328..61c05e6ff0a 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -1170,7 +1170,6 @@ static void test_CommandLine(void) get_file_name(resfile); sprintf(buffer2, "\"%s \" process dump \"%s\"", buffer, resfile); ret = CreateProcessA(NULL, buffer2, NULL, NULL, FALSE, 0L, NULL, NULL, &startup, &info); - todo_wine ok(ret, "CreateProcess (%s) failed : %ld\n", buffer, GetLastError()); if (info.hProcess) { diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 3656e40280d..b6e48853f49 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -87,10 +87,20 @@ static WCHAR *get_file_name( WCHAR *cmdline, WCHAR *buffer, DWORD buflen ) if (cmdline[0] == '"' && (p = wcschr( cmdline + 1, '"' ))) { - int len = p - cmdline - 1; + int len; + /* trim spaces in quotes */ + const WCHAR* start = cmdline + 1; + const WCHAR* end = p - 1; + while (*end == ' ' && end > start) end--; + if (end < start) + { + SetLastError( ERROR_INVALID_PARAMETER ); + return ret; + } + len = end - start + 1; /* extract the quoted portion as file name */ if (!(name = RtlAllocateHeap( GetProcessHeap(), 0, (len + 1) * sizeof(WCHAR) ))) return NULL; - memcpy( name, cmdline + 1, len * sizeof(WCHAR) ); + memcpy( name, start, len * sizeof(WCHAR) ); name[len] = 0; if (!find_exe_file( name, buffer, buflen )) goto done; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9882
On Fri Jan 16 05:05:06 2026 +0000, Yeshun Ye wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/9882/diffs?diff_id=238196&start_sha=7b9f2f1d18a6ba3199f9200c2eee0d9ee6eadccb#4288ae9b2be59c5bb99d18623489058310e49164_634_630) done
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9882#note_127163
On Fri Jan 16 05:05:06 2026 +0000, Yeshun Ye wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/9882/diffs?diff_id=238196&start_sha=7b9f2f1d18a6ba3199f9200c2eee0d9ee6eadccb#4288ae9b2be59c5bb99d18623489058310e49164_642_630) done
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9882#note_127164
On Fri Jan 16 05:05:06 2026 +0000, Yeshun Ye wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/9882/diffs?diff_id=238196&start_sha=7b9f2f1d18a6ba3199f9200c2eee0d9ee6eadccb#4288ae9b2be59c5bb99d18623489058310e49164_641_630) done
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9882#note_127165
On Fri Jan 16 05:05:06 2026 +0000, Yeshun Ye wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/9882/diffs?diff_id=238196&start_sha=7b9f2f1d18a6ba3199f9200c2eee0d9ee6eadccb#361f2e2f32e8486d0267db3f58ea3b45e8904666_95_94) done
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9882#note_127166
eric pouech (@epo) commented about dlls/kernelbase/process.c:
if (cmdline[0] == '"' && (p = wcschr( cmdline + 1, '"' ))) { - int len = p - cmdline - 1; + int len; + /* trim spaces in quotes */ + const WCHAR* start = cmdline + 1; + const WCHAR* end = p - 1; + while (*end == ' ' && end > start) end--; + if (end < start)
doesn't look good... 'end' can never be \< start (from line above), at worst it's == start advice: I'd rather write it as (not tested) `int len = p - cmdline - 1` `while (len && cmdline[1 + len - 1] == L' ') len--;` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9882#note_127186
participants (3)
-
eric pouech (＠epo) -
Yeshun Ye (＠yeyeshun)
-
YeshunYe