Use modern, secure, Windows API for dlopen

Internals & Design
1

Changing dlopen to use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS rather than LOAD_WITH_ALTERED_SEARCH_PATH

Currently, we use the windows “Alternate Search Order” to find DLLs. I propose that we use a new mechanism that is extensible via the Windows API.

Currently we use LOAD_WITH_ALTERED_SEARCH_PATH

On Windows, we currently use LOAD_WITH_ALTERED_SEARCH_PATH with LoadLibraryExW:

The Alternate Search Order for Desktop Applications is only extensible by appending the PATH environment variable.

Proposal: Use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR

After reading through the documentation on LoadLibraryExW when researching an issue GR.jl, I think we should use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR.

If we use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS then we could use AddDllDirectory to add directories to the search path. LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR also allows us to use the directory where the DLL is located.

The minimum version supported is Windows 7 with KB2533623: Insecure library loading could allow remote code execution.

Background reading: Cpython bpo36085, github 80266

Since Python 3.8, ctypes uses this approach:

https://bugs.python.org/issue36085

4 Likes
2

Have you pinged @jameson about this?

3

No, I have not pinged Jameson. I mentioned it in Slack #internals. I thought he might see it from there. But you’ve pinged him now. Hi Jameson!

4

That cpython thread was an odd read. They repeatedly noted this was a concerning breaking change in behavior, but then just gave up and merged it anyways. They also made no citation for their repeated claim that it is “safer”. It even ended up in the documentation that this is now made “safer” by this breaking change. They also claimed that Windows deprecated the current API, so switching is required (the linked documentation from Microsoft does not mention this), to this “safer” version. But this change is not safer—you cannot make things safer by disabling a capability the user already proved they have. If the OS fails to correctly check permissions on the user, there is little point to worry about whether the application might be exploited to also abuse the hole already required to exploit the application.

A few small takeaways is that Microsoft may have added a bug in Win 7sp1 with relative directory handling (while fixing an actual secure issue), so GetFullPathNameW may be required now

Also, we could disable searching the current directory entirely if we like. We could also attempt to make any AddDllDirectory calls affect all LoadLibrary calls. It is unclear if either of these are desirable.

In Julia, most of this has been effectively deprecated by JLL files though

4 Likes
5

Part of the reason I started to investigate this mechanism is because we are currently reliant on setting environment variables to communicate with GR:

GR uses its own plugin system and loads dynamic libraries itself:

Here we communicate the location of the GR binaries via the GRDIR.

My thought is that perhaps we could use this new API to communicate multiple directories to GR.

In trying out this API, I found something unpleasant. If someone calls SetDefaultDllDirectories with
LOAD_LIBRARY_SEARCH_DEFAULT_DIRS, then it causes LoadLibraryExW to fail with LOAD_WITH_ALTERED_SEARCH_PATH.

julia> begin
           wchar = append!(transcode(UInt16, raw"libdSFMT.dll"), 0x0000)
           LOAD_LIBRARY_SEARCH_DEFAULT_DIRS = 0x1000
           LOAD_WITH_ALTERED_SEARCH_PATH = 0x8
           status = @ccall "kernel32".SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_DEFAULT_DIRS::UInt32)::Bool # GR.jl v0.70.1
           handle = @ccall "kernel32".LoadLibraryExW(wchar::Ptr{UInt16}, C_NULL::Ptr{Nothing}, LOAD_WITH_ALTERED_SEARCH_PATH::UInt32)::Ptr{Nothing} # Julia
           if handle == C_NULL
               println(Libc.FormatMessage())
           end
           handle
       end
The parameter is incorrect. 
Ptr{Nothing} @0x0000000000000000

Everything loads fine if we use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS.

julia> begin
           wchar = append!(transcode(UInt16, raw"libdSFMT.dll"), 0x0000)
           LOAD_LIBRARY_SEARCH_DEFAULT_DIRS = 0x1000
           LOAD_WITH_ALTERED_SEARCH_PATH = 0x8
           status = @ccall "kernel32".SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_DEFAULT_DIRS::UInt32)::Bool
           handle = @ccall "kernel32".LoadLibraryExW(wchar::Ptr{UInt16}, C_NULL::Ptr{Nothing}, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS::UInt32)::Ptr{Nothing}        
           if handle == C_NULL
               println(Libc.FormatMessage())
           end
           handle
       end
Ptr{Nothing} @0x0000000000e20000

The main lesson from this is that we should not use SetDefaultDllDirectories since this causes Julia’s current use of LoadLibraryExW to fail when used with LOAD_WITH_ALTERED_SEARCH_PATH. The other thought is that using LOAD_WITH_ALTERED_SEARCH_PATH is fragile. Loading some plugin that invokes SetDefaultDllDirectories will cause Julia to be unable to dynamically load other dynamic libraries.

To make dynamic library loading more robust and and to take advantage of AddDllDirectory perhaps we could try to use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS with LoadLibraryExW if loading with LOAD_WITH_ALTERED_SEARCH_PATH fails. That might look as follows.

    HANDLE lib = LoadLibraryExW(wfilename, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
    if(lib == NULL)
    {
        lib = LoadLibraryExW(wfilename, NULL, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS);
    }

Essentially we try to load using LOAD_WITH_ALTERED_SEARCH_PATH. If that fails, then we try LOAD_LIBRARY_SEARCH_DEFAULT_DIRS.

LOAD_LIBRARY_SEARCH_DEFAULT_DIRS is a combination of the following:

  1. LOAD_LIBRARY_SEARCH_APPLICATION_DIR
  2. LOAD_LIBRARY_SEARCH_USER_DIRS (e.g. directories set with AddDllDirectory)
  3. LOAD_LIBRARY_SEARCH_SYSTEM32

Alternatively, the contingency loading could be restricted to LOAD_LIBRARY_SEARCH_USER_DIRS.

2 Likes
6

I created a reduced pull request here:

The pull request makes use of LOAD_LIBRARY_SEARCH_USER_DIRS if loading with LOAD_WITH_ALTERED_SEARCH_PATH fails.