Repository: go
Issue: runtime, syscall, x/sys/windows: unexpected DLL loading behavior
Labels: Security,OS-Windows,NeedsFix,compiler/runtime

In specific scenarios binaries on Windows can exhibit unexpected behavior when retrieving procedure addresses from system DLLs.

In general we take care to only load system DLLs from controlled locations. In particular we attempt to restrict the DLL search path by using LoadLibraryExW using the LOAD_LIBRARY_SEARCH_SYSTEM32 flag which dictates that "%windows%\system32 is searched for the DLL and its dependencies" (according to the [LoadLibraryExW doc](https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexw)). It is expected that when we load a procedure address (using GetProcAddress) for a function which is forwarded to another DLL (such as SystemFunction036, which is loaded from advapi32.dll, but defined in cryptbase.dll), the search path for that DLL is restricted in the same way that the search for the initial DLL is restricted.

However, when the initial DLL is already loaded, e.g. because it has been explicitly linked against the binary, and the Windows loader has already loaded it, the search path for a dependent/forwarded DLL (which has not already been loaded) is _not_ restricted as expected, and seems to fallback to a path similar to that dictated by LOAD_LIBRARY_SEARCH_DEFAULT_DIRS. Notably the new search path contains the application directory (the directory in which the executed binary resides, which, it should be noted, may not be the current directory), which is unexpected. As such if the application directory contains a DLL which has the same name as the system DLL that is expected to be loaded, it will be loaded instead.

This is clearly a security issue, since it can result in loading an unexpected DLL, and violates a number of documented properties (especially around `syscall.LoadDLL/LazyDLL` and the golang.org/x/sys/windows variants). That said, we do not consider it a PRIVATE track security issue per the [security policy](https://go.dev/security/policy#private) as the application directory is unlikely to be an attacker controlled directory, and there is no obvious solution to the problem which does not break a number of valid use cases for loading user DLLs.

While working on this issue we considered a number of possible solutions to the problem, in particular calling `SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32)` during runtime initialization, before loading the syscalls we need for the runtime, and then calling `SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_DEFAULT_DIRS)` before passing off to the user. This inherently breaks a number of use cases though, as it changes the [default DLL search order precedence](https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order#alternate-search-order-for-unpackaged-apps) for _all_ DLLs, in particular removing the current directory from the search path. It also still doesn't protect usages of `syscall.LoadDLL/LazyDLL` et al by users, outside of the runtime operations. It is still possible we will take this approach in a future release, but more work needs to be done to figure out the correct way to implement it. Another option may be to provide a mode which severely locks down the DLL search path, to only LOAD_LIBRARY_SEARCH_SYSTEM32, but this would need to be an opt-in solution.

Thanks to Nasreddine Bencherchali (@nas_bench) and Max Altgelt (@secDre4mer) of Nextron Systems for reporting this.