Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows: making library loading work after SetDefaultDllDirectories() call (#1413) #1614

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

trespasserw
Copy link
Contributor

When a client doesn't specify open options, JNA defaults to LOAD_WITH_ALTERED_SEARCH_PATH (dispatch.c#L54). According to LoadLibraryEx documentation, this results in an undefined behavior when a library name is given as a relative path.

In a "normal" mode, this is not a problem: Windows just ignores the flag and carries on, but if an application tries to improve DLL loading security and restricts the DLL search path by calling SetDefaultDllDirectories() (docs), calling LoadLibraryEx(relative_path, ..., LOAD_WITH_ALTERED_SEARCH_PATH) results in ERROR_INVALID_PARAMETER.

A simple reproducer in C, just in case:

#include <stdio.h>
#include <Windows.h>

int main() {
  if (!SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_DEFAULT_DIRS)) {
    printf("SetDefaultDllDirectories: 0x%lx\n", GetLastError());
    return 1;
  }

  HMODULE w = LoadLibraryExW(L"WtsApi32", NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
  if (w == NULL) {
    printf("LoadLibraryEx: 0x%lx\n", GetLastError());
    return 2;
  }

  printf("handle=%p\n", w);

  return 0;
}

The PR fixes the issue by setting default open options to 0 when a relative path is specified.

@trespasserw
Copy link
Contributor Author

trespasserw commented Jul 2, 2024

Admittedly, fixing the problem in Java_com_sun_jna_Native_open() would be more correct - at the cost of an additional Win32 call (for checking whether a given path is relative) and recompilation of Windows binaries.

Copy link
Member

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think I understand what is the intention here. However, I think this needs a different/updated approach.

Native#loadLibrary makes multiple iterations to find the library and indeed tries to build a full path before passing it to the native library loader. Only if that fails, then it will let the OS takeover and let that search for the library.

That also means, that after each call to Native#findLibraryPath (or before each Native#open call) you might be faced with a full path and thus the change would be a behavioral change and not just undefined behavior.

What I'm missing is why you can't use the option to pass the openflags for the library when you load it:

DummyLib INSTANCE = Native.load("dummylib", DummyLib.class, singletonMap(Library.OPTION_OPEN_FLAGS, 0));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants