Skip to content

Commit

Permalink
[java-interop, Java.Interop] Securely load native libs
Browse files Browse the repository at this point in the history
Fixes: dotnet#676

Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide

The current security guidance is that the
[`System.Runtime.InteropServices.DefaultDllImportSearchPathsAttribute`][0]
attribute should be placed either on the assembly or on `[DllImport]`
methods to control and constrain where [`LoadLibraryEx()`][1] will look
for native libraries, in particular to *prevent* looking for native
libraries within e.g. the current working directory or `%PATH%` or any
other "attacker-controlled" location.

Update `Java.Interop.dll` and `Java.Runtime.Environment.dll` so that
the [`DllImportSearchPath`][2] values `AssemblyDirectory` and
`SafeDirectories` are used:

  * `AssemblyDirectory`: "include the directory that contains the
    assembly itself, and search that directory first."

  * `SafeDirectories`: "Include the application directory, the
    `%WinDir%\System32` directory, and user directories in the DLL
    search path.

Additionally, update `src/java-interop` so that instead of requiring
the use of [**dlopen**(3)] on Windows, the following new exports are
added to support loading native libraries and resolving symbols from
those native libraries:

	void*   java_interop_load_library (const char *path, unsigned int flags, char **error);
	void*   java_interop_get_symbol_address (void* library, const char *symbol, char **error);
	int     java_interop_close_library (void* library, char **error);

On Windows, `java_interop_load_library()` will use
[`LoadLibraryEx()`][4] to load libraries from a constrained set of
directories:

  * `LOAD_LIBRARY_SEARCH_APPLICATION_DIR`: "the application's
    installation directory is searched for the DLL and its dependencies"

  * `LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR`: "the directory that contains
    the DLL is temporarily added to the beginning of the list of
    directories that are searched for the DLL's dependencies."

  * `LOAD_LIBRARY_SEARCH_USER_DIRS`: "directories added using the
    `AddDllDirectory()` or the `SetDllDirectory()` function are searched
    for the DLL and its dependencies"

In order to simplify the introduction of
`java_interop_load_library()`, start *requiring* the presence of the
symbols `mono_thread_get_managed_id` and `mono_thread_get_name_utf8`.
These symbols have been present within Mono for ages at this point,
and requiring means we don't need to support `dlopen(NULL)` semantics.

Update the `@(ClInclude)` item group and `BuildMac` and related targets
so that we properly rebuild things when e.g. `java-interop-dlfcn.h`
changes, as would "normally" be expected.

Finally, the continued use of `MONO_API` and other macros causes
"weird" compiler issues when integrating with xamarin-android.
Replace `MONO_API`/etc. use with `JAVA_INTEROP_API`/etc. instead.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.defaultdllimportsearchpathsattribute?view=netcore-3.1
[1]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa?redirectedfrom=MSDN
[2]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.dllimportsearchpath?view=netcore-3.1
[3]: https://linux.die.net/man/3/dlopen
[4]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa
  • Loading branch information
jonpryor committed Aug 20, 2020
1 parent 007b35b commit 001a684
Show file tree
Hide file tree
Showing 13 changed files with 316 additions and 145 deletions.
2 changes: 2 additions & 0 deletions src/Java.Interop/Java.Interop/JniRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
using System.Runtime.InteropServices;
using System.Threading;

[assembly: DefaultDllImportSearchPathsAttribute (DllImportSearchPath.SafeDirectories | DllImportSearchPath.AssemblyDirectory)]

namespace Java.Interop
{
delegate int DestroyJavaVMDelegate (IntPtr javavm);
Expand Down
2 changes: 2 additions & 0 deletions src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using System.Runtime.InteropServices;
using System.Threading;

[assembly: DefaultDllImportSearchPathsAttribute (DllImportSearchPath.SafeDirectories | DllImportSearchPath.AssemblyDirectory)]

namespace Java.Interop {

struct JavaVMInitArgs {
Expand Down
183 changes: 183 additions & 0 deletions src/java-interop/java-interop-dlfcn.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
#include "java-interop.h"
#include "java-interop-dlfcn.h"
#include "java-interop-util.h"

#ifdef WINDOWS
#include <libloaderapi.h>
#include <winerror.h>
#include <wtypes.h>
#include <winbase.h>
#else
#include <dlfcn.h>
#endif

static char *
_get_last_dlerror ()
{
#ifdef WINDOWS

DWORD error = GetLastError ();
if (error == ERROR_SUCCESS /* 0 */) {
return nullptr;
}

wchar_t *buf = nullptr;

DWORD size = FormatMessageW (
/* dwFlags */ FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |FORMAT_MESSAGE_IGNORE_INSERTS,
/* lpSource */ NULL,
/* dwMessageId */ error,
/* dwLanguageId */ MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
/* lpBuffer */ (LPWSTR) &buf,
/* nSize */ 0,
/* Arguments */ NULL
);
if (size == 0)
return nullptr;

char *message = utf16_to_utf8 (buf);
LocalFree (buf);

return message;

#else // ndef WINDOWS

return java_interop_strdup (dlerror ());

#endif // ndef WINDOWS
}

static void
_free_error (char **error)
{
if (error == nullptr)
return;
java_interop_free (*error);
*error = nullptr;
}

static void
_set_error (char **error, const char *message)
{
if (error == nullptr)
return;
*error = java_interop_strdup (message);
}

static void
_set_error_to_last_error (char **error)
{
if (error == nullptr)
return;
*error = _get_last_dlerror ();
}

void*
java_interop_load_library (const char *path, unsigned int flags, char **error)
{
_free_error (error);
if (path == nullptr) {
_set_error (error, "path=nullptr is not supported");
return nullptr;
}
if (flags != 0) {
_set_error (error, "flags has unsupported value");
return nullptr;
}

void *handle = nullptr;

#ifdef WINDOWS

wchar_t *wpath = utf8_to_utf16 (path);
if (path == nullptr) {
_set_error (error, "could not convert path to UTF-16");
return nullptr;
}
HMODULE module = LoadLibraryExW (
/* lpLibFileName */ wpath,
/* hFile */ nullptr,
/* dwFlags */ LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_USER_DIRS
);
java_interop_free (wpath);

handle = reinterpret_cast<void*>(module);

#else // ndef WINDOWS

handle = dlopen (path, RTLD_LAZY | RTLD_LOCAL);

#endif // ndef WINDOWS

if (handle == nullptr) {
_set_error_to_last_error (error);
}

return handle;
}

void*
java_interop_get_symbol_address (void *library, const char *symbol, char **error)
{
_free_error (error);

if (library == nullptr) {
_set_error (error, "library=nullptr");
return nullptr;
}
if (symbol == nullptr) {
_set_error (error, "symbol=nullptr");
return nullptr;
}

void *address = nullptr;

#ifdef WINDOWS

HMODULE module = reinterpret_cast<HMODULE>(library);
FARPROC a = GetProcAddress (module, symbol);
address = reinterpret_cast<void*>(a);

#else // ndef WINDOWS

address = dlsym (library, symbol);

#endif // ndef WINDOWS

if (address == nullptr) {
_set_error_to_last_error (error);
}

return address;
}

int
java_interop_close_library (void* library, char **error)
{
_free_error (error);
if (library == nullptr) {
_set_error (error, "library=nullptr");
return JAVA_INTEROP_LIBRARY_INVALID_PARAM;
}

int r = 0;

#ifdef WINDOWS
HMODULE h = reinterpret_cast<HMODULE>(library);
BOOL v = FreeLibrary (h);
if (!v) {
r = JAVA_INTEROP_LIBRARY_CLOSE_FAILED;
}
#else // ndef WINDOWS
r = dlclose (library);
if (r != 0) {
r = JAVA_INTEROP_LIBRARY_CLOSE_FAILED;
}
#endif // ndef WINDOWS

if (r != 0) {
_set_error_to_last_error (error);
}

return r;
}
20 changes: 20 additions & 0 deletions src/java-interop/java-interop-dlfcn.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#ifndef INC_JAVA_INTEROP_DLFCN_H
#define INC_JAVA_INTEROP_DLFCN_H

#include "java-interop.h"

JAVA_INTEROP_BEGIN_DECLS

// Possible error codes from java_interop_close_library
constexpr int JAVA_INTEROP_LIBRARY_FAILED = -1000;
constexpr int JAVA_INTEROP_LIBRARY_CLOSE_FAILED = JAVA_INTEROP_LIBRARY_FAILED-1;
constexpr int JAVA_INTEROP_LIBRARY_INVALID_PARAM = JAVA_INTEROP_LIBRARY_FAILED-2;


JAVA_INTEROP_API void* java_interop_load_library (const char *path, unsigned int flags, char **error);
JAVA_INTEROP_API void* java_interop_get_symbol_address (void* library, const char *symbol, char **error);
JAVA_INTEROP_API int java_interop_close_library (void* library, char **error);

JAVA_INTEROP_END_DECLS

#endif /* INC_JAVA_INTEROP_DLFCN_H */
40 changes: 4 additions & 36 deletions src/java-interop/java-interop-gc-bridge-mono.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
#include "logger.h"
#endif /* !defined (ANDROID) */

#include <dlfcn.h>

#if defined (ANDROID) || defined (XAMARIN_ANDROID_DYLIB_MONO)
using namespace xamarin::android;
#endif
Expand Down Expand Up @@ -76,23 +74,6 @@ struct JavaInteropGCBridge {
int gref_cleanup, lref_cleanup;
};

typedef char* (*MonoThreadGetNameUtf8)(MonoThread*);
typedef int32_t (*MonoThreadGetManagedId)(MonoThread*);

static MonoThreadGetManagedId _mono_thread_get_managed_id;
static MonoThreadGetNameUtf8 _mono_thread_get_name_utf8;

static void
lookup_optional_mono_thread_functions (void)
{
void *h = dlopen (NULL, RTLD_LAZY);
if (!h)
return;

_mono_thread_get_managed_id = reinterpret_cast<MonoThreadGetManagedId>(dlsym (h, "mono_thread_get_managed_id"));
_mono_thread_get_name_utf8 = reinterpret_cast<MonoThreadGetNameUtf8>(dlsym (h, "mono_thread_get_name_utf8"));
}

static jobject
lref_to_gref (JNIEnv *env, jobject lref)
{
Expand Down Expand Up @@ -275,8 +256,6 @@ java_interop_gc_bridge_new (JavaVM *jvm)
}
#endif /* defined (XAMARIN_ANDROID_DYLIB_MONO) */

lookup_optional_mono_thread_functions ();

JavaInteropGCBridge bridge = {0};

bridge.jvm = jvm;
Expand Down Expand Up @@ -1237,26 +1216,15 @@ gc_is_bridge_object (MonoObject *object)
static char *
get_thread_name (void)
{
if (_mono_thread_get_name_utf8) {
MonoThread *thread = mono_thread_current ();
return _mono_thread_get_name_utf8 (thread);
}
return strdup ("finalizer");
MonoThread *thread = mono_thread_current ();
return mono_thread_get_name_utf8 (thread);
}

static int64_t
get_thread_id (void)
{
if (_mono_thread_get_managed_id) {
MonoThread *thread = mono_thread_current ();
return _mono_thread_get_managed_id (thread);
}
#if __linux__
int64_t tid = (int64_t)((pid_t)syscall(SYS_gettid));
#else
int64_t tid = (int64_t) pthread_self ();
#endif
return tid;
MonoThread *thread = mono_thread_current ();
return mono_thread_get_managed_id (thread);
}

static void
Expand Down
54 changes: 27 additions & 27 deletions src/java-interop/java-interop-gc-bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,41 +19,41 @@ struct JavaInterop_System_RuntimeTypeHandle {
void *value;
};

MONO_API JavaInteropGCBridge *java_interop_gc_bridge_get_current (void);
MONO_API int java_interop_gc_bridge_set_current_once (JavaInteropGCBridge *bridge);
JAVA_INTEROP_API JavaInteropGCBridge *java_interop_gc_bridge_get_current (void);
JAVA_INTEROP_API int java_interop_gc_bridge_set_current_once (JavaInteropGCBridge *bridge);

MONO_API JavaInteropGCBridge *java_interop_gc_bridge_new (JavaVM *jvm);
MONO_API int java_interop_gc_bridge_free (JavaInteropGCBridge *bridge);
JAVA_INTEROP_API JavaInteropGCBridge *java_interop_gc_bridge_new (JavaVM *jvm);
JAVA_INTEROP_API int java_interop_gc_bridge_free (JavaInteropGCBridge *bridge);

MONO_API int java_interop_gc_bridge_register_hooks (JavaInteropGCBridge *bridge, int weak_ref_kind);
MONO_API int java_interop_gc_bridge_wait_for_bridge_processing (JavaInteropGCBridge *bridge);
JAVA_INTEROP_API int java_interop_gc_bridge_register_hooks (JavaInteropGCBridge *bridge, int weak_ref_kind);
JAVA_INTEROP_API int java_interop_gc_bridge_wait_for_bridge_processing (JavaInteropGCBridge *bridge);

MONO_API int java_interop_gc_bridge_add_current_app_domain (JavaInteropGCBridge *bridge);
MONO_API int java_interop_gc_bridge_remove_current_app_domain (JavaInteropGCBridge *bridge);
JAVA_INTEROP_API int java_interop_gc_bridge_add_current_app_domain (JavaInteropGCBridge *bridge);
JAVA_INTEROP_API int java_interop_gc_bridge_remove_current_app_domain (JavaInteropGCBridge *bridge);

MONO_API int java_interop_gc_bridge_set_bridge_processing_field (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle, char *field_name);
MONO_API int java_interop_gc_bridge_register_bridgeable_type (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle);
MONO_API int java_interop_gc_bridge_enable (JavaInteropGCBridge *bridge, int enable);
JAVA_INTEROP_API int java_interop_gc_bridge_set_bridge_processing_field (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle, char *field_name);
JAVA_INTEROP_API int java_interop_gc_bridge_register_bridgeable_type (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle);
JAVA_INTEROP_API int java_interop_gc_bridge_enable (JavaInteropGCBridge *bridge, int enable);

MONO_API int java_interop_gc_bridge_get_gref_count (JavaInteropGCBridge *bridge);
MONO_API int java_interop_gc_bridge_get_weak_gref_count (JavaInteropGCBridge *bridge);
JAVA_INTEROP_API int java_interop_gc_bridge_get_gref_count (JavaInteropGCBridge *bridge);
JAVA_INTEROP_API int java_interop_gc_bridge_get_weak_gref_count (JavaInteropGCBridge *bridge);

MONO_API int java_interop_gc_bridge_lref_set_log_file (JavaInteropGCBridge *bridge, const char *gref_log_file);
MONO_API FILE* java_interop_gc_bridge_lref_get_log_file (JavaInteropGCBridge *bridge);
MONO_API int java_interop_gc_bridge_lref_set_log_level (JavaInteropGCBridge *bridge, int level);
MONO_API void java_interop_gc_bridge_lref_log_message (JavaInteropGCBridge *bridge, int level, const char *message);
MONO_API void java_interop_gc_bridge_lref_log_new (JavaInteropGCBridge *bridge, int lref_count, jobject curHandle, char curType, jobject newHandle, char newType, const char *thread_name, int64_t thread_id, const char *from);
MONO_API void java_interop_gc_bridge_lref_log_delete (JavaInteropGCBridge *bridge, int lref_count, jobject handle, char type, const char *thread_name, int64_t thread_id, const char *from);
JAVA_INTEROP_API int java_interop_gc_bridge_lref_set_log_file (JavaInteropGCBridge *bridge, const char *gref_log_file);
JAVA_INTEROP_API FILE* java_interop_gc_bridge_lref_get_log_file (JavaInteropGCBridge *bridge);
JAVA_INTEROP_API int java_interop_gc_bridge_lref_set_log_level (JavaInteropGCBridge *bridge, int level);
JAVA_INTEROP_API void java_interop_gc_bridge_lref_log_message (JavaInteropGCBridge *bridge, int level, const char *message);
JAVA_INTEROP_API void java_interop_gc_bridge_lref_log_new (JavaInteropGCBridge *bridge, int lref_count, jobject curHandle, char curType, jobject newHandle, char newType, const char *thread_name, int64_t thread_id, const char *from);
JAVA_INTEROP_API void java_interop_gc_bridge_lref_log_delete (JavaInteropGCBridge *bridge, int lref_count, jobject handle, char type, const char *thread_name, int64_t thread_id, const char *from);

MONO_API int java_interop_gc_bridge_gref_set_log_file (JavaInteropGCBridge *bridge, const char *gref_log_file);
MONO_API FILE* java_interop_gc_bridge_gref_get_log_file (JavaInteropGCBridge *bridge);
MONO_API int java_interop_gc_bridge_gref_set_log_level (JavaInteropGCBridge *bridge, int level);
MONO_API void java_interop_gc_bridge_gref_log_message (JavaInteropGCBridge *bridge, int level, const char *message);
MONO_API int java_interop_gc_bridge_gref_log_new (JavaInteropGCBridge *bridge, jobject curHandle, char curType, jobject newHandle, char newType, const char *thread_name, int64_t thread_id, const char *from);
MONO_API int java_interop_gc_bridge_gref_log_delete (JavaInteropGCBridge *bridge, jobject handle, char type, const char *thread_name, int64_t thread_id, const char *from);
JAVA_INTEROP_API int java_interop_gc_bridge_gref_set_log_file (JavaInteropGCBridge *bridge, const char *gref_log_file);
JAVA_INTEROP_API FILE* java_interop_gc_bridge_gref_get_log_file (JavaInteropGCBridge *bridge);
JAVA_INTEROP_API int java_interop_gc_bridge_gref_set_log_level (JavaInteropGCBridge *bridge, int level);
JAVA_INTEROP_API void java_interop_gc_bridge_gref_log_message (JavaInteropGCBridge *bridge, int level, const char *message);
JAVA_INTEROP_API int java_interop_gc_bridge_gref_log_new (JavaInteropGCBridge *bridge, jobject curHandle, char curType, jobject newHandle, char newType, const char *thread_name, int64_t thread_id, const char *from);
JAVA_INTEROP_API int java_interop_gc_bridge_gref_log_delete (JavaInteropGCBridge *bridge, jobject handle, char type, const char *thread_name, int64_t thread_id, const char *from);

MONO_API int java_interop_gc_bridge_weak_gref_log_new (JavaInteropGCBridge *bridge, jobject curHandle, char curType, jobject newHandle, char newType, const char *thread_name, int64_t thread_id, const char *from);
MONO_API int java_interop_gc_bridge_weak_gref_log_delete (JavaInteropGCBridge *bridge, jobject handle, char type, const char *thread_name, int64_t thread_id, const char *from);
JAVA_INTEROP_API int java_interop_gc_bridge_weak_gref_log_new (JavaInteropGCBridge *bridge, jobject curHandle, char curType, jobject newHandle, char newType, const char *thread_name, int64_t thread_id, const char *from);
JAVA_INTEROP_API int java_interop_gc_bridge_weak_gref_log_delete (JavaInteropGCBridge *bridge, jobject handle, char type, const char *thread_name, int64_t thread_id, const char *from);

JAVA_INTEROP_END_DECLS

Expand Down
Loading

0 comments on commit 001a684

Please sign in to comment.