[PATCH v4 1/2] kernel32: add tests for LdrGetDllPath with LOAD_WITH_ALTERED_SEARCH_PATH
Add tests for various paths and undefined behavior with regards to LdrGetDllPath with LOAD_ALTERED_SEARCH_PATH and relative module names. Signed-off-by: Nick Fox <nick(a)foxsec.net> --- v4: add another test case and fix todo_wine usage --- dlls/kernel32/tests/path.c | 101 +++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/dlls/kernel32/tests/path.c b/dlls/kernel32/tests/path.c index f49af6d5bfe..b2a6d5acbda 100644 --- a/dlls/kernel32/tests/path.c +++ b/dlls/kernel32/tests/path.c @@ -2654,6 +2654,107 @@ static void test_LdrGetDllPath(void) ok( !lstrcmpW( path, L"\\\\?\\c:" ), "got %s expected \\\\?\\c:\n", wine_dbgstr_w(path)); pRtlReleasePath( path ); + lstrcpyW( buffer, fooW ); + ret = pLdrGetDllPath( buffer, LOAD_WITH_ALTERED_SEARCH_PATH, &path, &unknown_ptr ); + ok( !ret, "LdrGetDllPath failed %x\n", ret ); + ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); + build_search_path( buffer, ARRAY_SIZE(buffer), NULL, TRUE ); + todo_wine + ok( path_equal( path, buffer ), "got %s expected %s\n", wine_dbgstr_w(path), wine_dbgstr_w(buffer)); + pRtlReleasePath( path ); + + lstrcpyW( buffer, L"temp" ); + p = buffer + lstrlenW( buffer ); + *p++ = '/'; + lstrcpyW( p, fooW ); + ret = pLdrGetDllPath( buffer, LOAD_WITH_ALTERED_SEARCH_PATH, &path, &unknown_ptr ); + ok( !ret, "LdrGetDllPath failed %x\n", ret ); + ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); + build_search_path( buffer, ARRAY_SIZE(buffer), NULL, TRUE ); + todo_wine + ok( path_equal( path, buffer ), "got %s expected %s\n", wine_dbgstr_w(path), wine_dbgstr_w(buffer)); + pRtlReleasePath( path ); + + lstrcpyW( buffer, L".\\foo\\foooo" ); + ret = pLdrGetDllPath( buffer, LOAD_WITH_ALTERED_SEARCH_PATH, &path, &unknown_ptr ); + ok( !ret, "LdrGetDllPath failed %x\n", ret ); + ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); + lstrcpyW( buffer, L".\\foo" ); + p = buffer + lstrlenW( buffer ); + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer) - 2; /* remove "32" */ + *p++ = ';'; + GetWindowsDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + *p++ = '.'; + *p++ = ';'; + GetEnvironmentVariableW( pathW, p, buffer + ARRAY_SIZE(buffer) - p ); + ok( path_equal( path, buffer ), "got %s expected %s\n", wine_dbgstr_w(path), wine_dbgstr_w(buffer)); + pRtlReleasePath( path ); + + lstrcpyW( buffer, L"temp" ); + p = buffer + lstrlenW( buffer ); + *p++ = '\\'; + lstrcpyW( p, fooW ); + ret = pLdrGetDllPath( buffer, LOAD_WITH_ALTERED_SEARCH_PATH, &path, &unknown_ptr ); + ok( !ret, "LdrGetDllPath failed %x\n", ret ); + ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); + lstrcpyW( buffer, L"temp" ); + p = buffer + lstrlenW( buffer ); + *p++ = '\\'; + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer) - 2; /* remove "32" */ + *p++ = ';'; + GetWindowsDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + *p++ = '.'; + *p++ = ';'; + GetEnvironmentVariableW( pathW, p, buffer + ARRAY_SIZE(buffer) - p ); + ok( path_equal( path, buffer ), "got %s expected %s\n", wine_dbgstr_w(path), wine_dbgstr_w(buffer)); + pRtlReleasePath( path ); + + lstrcpyW( buffer, L"c:\\temp" ); + p = buffer + lstrlenW(buffer); + *p++ = '\\'; + lstrcpyW( p, fooW ); + ret = pLdrGetDllPath( buffer, LOAD_WITH_ALTERED_SEARCH_PATH, &path, &unknown_ptr ); + ok( !ret, "LdrGetDllPath failed %x\n", ret ); + ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); + lstrcpyW( buffer, L"c:\\temp" ); + p = buffer + lstrlenW( buffer ); + *p++ = '\\'; + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer) - 2; /* remove "32" */ + *p++ = ';'; + GetWindowsDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + *p++ = '.'; + *p++ = ';'; + GetEnvironmentVariableW( pathW, p, buffer + ARRAY_SIZE(buffer) - p ); + ok( path_equal( path, buffer ), "got %s expected %s\n", wine_dbgstr_w(path), wine_dbgstr_w(buffer)); + pRtlReleasePath( path ); + + lstrcpyW( buffer, fooW ); + ret = pLdrGetDllPath( buffer, LOAD_WITH_ALTERED_SEARCH_PATH | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR, &path, &unknown_ptr ); + ok( ret == STATUS_INVALID_PARAMETER, "got %x expected %x\n", ret, STATUS_INVALID_PARAMETER ); + ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); + pRtlReleasePath( path ); + lstrcpyW( buffer, dlldir ); p = buffer + lstrlenW(buffer); *p++ = '\\'; -- 2.33.0
Windows adds the executable directory to the path when LdrGetDllPath is called with LOAD_WITH_ALTERED_SEARCH_PATH and a relative module name, but only when the module name does not include directories. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=26350 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51821 Signed-off-by: Nick Fox <nick(a)foxsec.net> --- v4: add additional test case and fix todo_wine usage --- dlls/kernel32/tests/path.c | 2 -- dlls/ntdll/loader.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dlls/kernel32/tests/path.c b/dlls/kernel32/tests/path.c index b2a6d5acbda..63f5ab21fa5 100644 --- a/dlls/kernel32/tests/path.c +++ b/dlls/kernel32/tests/path.c @@ -2659,7 +2659,6 @@ static void test_LdrGetDllPath(void) ok( !ret, "LdrGetDllPath failed %x\n", ret ); ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); build_search_path( buffer, ARRAY_SIZE(buffer), NULL, TRUE ); - todo_wine ok( path_equal( path, buffer ), "got %s expected %s\n", wine_dbgstr_w(path), wine_dbgstr_w(buffer)); pRtlReleasePath( path ); @@ -2671,7 +2670,6 @@ static void test_LdrGetDllPath(void) ok( !ret, "LdrGetDllPath failed %x\n", ret ); ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); build_search_path( buffer, ARRAY_SIZE(buffer), NULL, TRUE ); - todo_wine ok( path_equal( path, buffer ), "got %s expected %s\n", wine_dbgstr_w(path), wine_dbgstr_w(buffer)); pRtlReleasePath( path ); diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 797d72aedb6..ca128308032 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -4208,7 +4208,7 @@ NTSTATUS WINAPI LdrGetDllPath( PCWSTR module, ULONG flags, PWSTR *path, PWSTR *u else { const WCHAR *dlldir = dll_directory.Length ? dll_directory.Buffer : NULL; - if (!(flags & LOAD_WITH_ALTERED_SEARCH_PATH)) + if (!(flags & LOAD_WITH_ALTERED_SEARCH_PATH) || !wcschr( module, L'\\' )) module = NtCurrentTeb()->Peb->ProcessParameters->ImagePathName.Buffer; status = get_dll_load_path( module, dlldir, dll_safe_mode, path ); } -- 2.33.0
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=100061 Your paranoid android. === w8adm (32 bit report) === kernel32: path: Timeout
Nick Fox <nick(a)foxsec.net> writes:
+ lstrcpyW( buffer, L"c:\\temp" ); + p = buffer + lstrlenW(buffer); + *p++ = '\\'; + lstrcpyW( p, fooW ); + ret = pLdrGetDllPath( buffer, LOAD_WITH_ALTERED_SEARCH_PATH, &path, &unknown_ptr ); + ok( !ret, "LdrGetDllPath failed %x\n", ret ); + ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); + lstrcpyW( buffer, L"c:\\temp" ); + p = buffer + lstrlenW( buffer ); + *p++ = '\\'; + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer) - 2; /* remove "32" */ + *p++ = ';'; + GetWindowsDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + *p++ = '.'; + *p++ = ';';
Please use the existing helper instead of duplicating all this code. -- Alexandre Julliard julliard(a)winehq.org
On 10/25/21 15:34, Alexandre Julliard wrote:
Nick Fox <nick(a)foxsec.net> writes:
+ lstrcpyW( buffer, L"c:\\temp" ); + p = buffer + lstrlenW(buffer); + *p++ = '\\'; + lstrcpyW( p, fooW ); + ret = pLdrGetDllPath( buffer, LOAD_WITH_ALTERED_SEARCH_PATH, &path, &unknown_ptr ); + ok( !ret, "LdrGetDllPath failed %x\n", ret ); + ok( !unknown_ptr, "unknown ptr %p\n", unknown_ptr ); + lstrcpyW( buffer, L"c:\\temp" ); + p = buffer + lstrlenW( buffer ); + *p++ = '\\'; + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + GetSystemDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer) - 2; /* remove "32" */ + *p++ = ';'; + GetWindowsDirectoryW( p, buffer + ARRAY_SIZE(buffer) - p ); + p = buffer + lstrlenW(buffer); + *p++ = ';'; + *p++ = '.'; + *p++ = ';'; Please use the existing helper instead of duplicating all this code.
-- Alexandre Julliard julliard(a)winehq.org
Thank you for the review and suggestion. Also apologies in advance if this email isn't formatted properly, this is my first time. Did you mean the build_search_path() function? I looked at it but it was subtly different than what I needed to do (related to the bug), and I wasn't comfortable defining a new one or adding complexity to the existing one by handling this edge case. For example, here are the results on Windows when I use build_search_path() instead of building it manually: path.c:2696: Test failed: got L".\\foo;C:\\WINDOWS\\SYSTEM32;C:\\WINDOWS\\system;C:\\WINDOWS;.;foo" expected L"C:\\Users\\nick\\Documents;C:\\WINDOWS\\system32;C:\\WINDOWS\\system;C:\\WINDOWS;.;foo" path.c:2723: Test failed: got L"temp\\;C:\\WINDOWS\\SYSTEM32;C:\\WINDOWS\\system;C:\\WINDOWS;.;foo" expected L"C:\\Users\\nick\\Documents;C:\\WINDOWS\\system32;C:\\WINDOWS\\system;C:\\WINDOWS;.;foo" path.c:2750: Test failed: got L"c:\\temp;C:\\WINDOWS\\SYSTEM32;C:\\WINDOWS\\system;C:\\WINDOWS;.;foo" expected L"C:\\Users\\nick\\Documents;C:\\WINDOWS\\system32;C:\\WINDOWS\\system;C:\\WINDOWS;.;foo" They all pass when building the path manually. Do you think it would be better to try and modify the existing helper function, or add a new one? If adding a new one, I wasn't sure what to call it. build_altered_search_path() or something like that maybe?
participants (4)
-
Alexandre Julliard -
Marvin -
Nick -
Nick Fox