Skip to content

Commit bf2c410

Browse files
jonpryorjonpryor
committed
[java-interop, Java.Interop] Securely load native libs
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
1 parent 007b35b commit bf2c410

File tree

13 files changed

+317
-145
lines changed

13 files changed

+317
-145
lines changed

‎src/Java.Interop/Java.Interop/JniRuntime.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
using System.Runtime.InteropServices;
1010
using System.Threading;
1111

12+
[assembly: DefaultDllImportSearchPathsAttribute (DllImportSearchPath.SafeDirectories | DllImportSearchPath.AssemblyDirectory)]
13+
1214
namespace Java.Interop
1315
{
1416
delegate int DestroyJavaVMDelegate (IntPtr javavm);

‎src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
using System.Runtime.InteropServices;
88
using System.Threading;
99

10+
[assembly: DefaultDllImportSearchPathsAttribute (DllImportSearchPath.SafeDirectories | DllImportSearchPath.AssemblyDirectory)]
11+
1012
namespace Java.Interop {
1113

1214
struct JavaVMInitArgs {

‎src/java-interop/java-interop-dlfcn.cc‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
#include "java-interop.h"
2+
#include "java-interop-dlfcn.h"
3+
#include "java-interop-util.h"
4+
5+
#ifdef WINDOWS
6+
#include <libloaderapi.h>
7+
#include <winerror.h>
8+
#include <wtypes.h>
9+
#include <winbase.h>
10+
#else
11+
#include <dlfcn.h>
12+
#endif
13+
14+
static char *
15+
_get_last_dlerror ()
16+
{
17+
#ifdef WINDOWS
18+
19+
DWORD error = GetLastError ();
20+
if (error == ERROR_SUCCESS /* 0 */) {
21+
return nullptr;
22+
}
23+
24+
wchar_t *buf = nullptr;
25+
26+
DWORD size = FormatMessageW (
27+
/* dwFlags */ FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
28+
/* lpSource */ NULL,
29+
/* dwMessageId */ error,
30+
/* dwLanguageId */ MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT),
31+
/* lpBuffer */ (LPWSTR) &buf,
32+
/* nSize */ 0,
33+
/* Arguments */ NULL
34+
);
35+
if (size == 0)
36+
return nullptr;
37+
38+
char *message = utf16_to_utf8 (buf);
39+
LocalFree (buf);
40+
41+
return message;
42+
43+
#else // ndef WINDOWS
44+
45+
return java_interop_strdup (dlerror ());
46+
47+
#endif // ndef WINDOWS
48+
}
49+
50+
static void
51+
_free_error (char **error)
52+
{
53+
if (error == nullptr)
54+
return;
55+
java_interop_free (*error);
56+
*error = nullptr;
57+
}
58+
59+
static void
60+
_set_error (char **error, const char *message)
61+
{
62+
if (error == nullptr)
63+
return;
64+
*error = java_interop_strdup (message);
65+
}
66+
67+
static void
68+
_set_error_to_last_error (char **error)
69+
{
70+
if (error == nullptr)
71+
return;
72+
*error = _get_last_dlerror ();
73+
}
74+
75+
void*
76+
java_interop_load_library (const char *path, unsigned int flags, char **error)
77+
{
78+
_free_error (error);
79+
if (path == nullptr) {
80+
_set_error (error, "path=nullptr is not supported");
81+
return nullptr;
82+
}
83+
if (flags != 0) {
84+
_set_error (error, "flags has unsupported value");
85+
return nullptr;
86+
}
87+
88+
void *handle = nullptr;
89+
90+
#ifdef WINDOWS
91+
92+
wchar_t *wpath = utf8_to_utf16 (path);
93+
if (wpath == nullptr) {
94+
_set_error (error, "could not convert path to UTF-16");
95+
return nullptr;
96+
}
97+
HMODULE module = LoadLibraryExW (
98+
/* lpLibFileName */ wpath,
99+
/* hFile */ nullptr,
100+
/* dwFlags */ LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_USER_DIRS
101+
);
102+
java_interop_free (wpath);
103+
104+
handle = reinterpret_cast<void*>(module);
105+
106+
#else // ndef WINDOWS
107+
108+
handle = dlopen (path, RTLD_LAZY | RTLD_LOCAL);
109+
110+
#endif // ndef WINDOWS
111+
112+
if (handle == nullptr) {
113+
_set_error_to_last_error (error);
114+
}
115+
116+
return handle;
117+
}
118+
119+
void*
120+
java_interop_get_symbol_address (void *library, const char *symbol, char **error)
121+
{
122+
_free_error (error);
123+
124+
if (library == nullptr) {
125+
_set_error (error, "library=nullptr");
126+
return nullptr;
127+
}
128+
if (symbol == nullptr) {
129+
_set_error (error, "symbol=nullptr");
130+
return nullptr;
131+
}
132+
133+
void *address = nullptr;
134+
135+
#ifdef WINDOWS
136+
137+
HMODULE module = reinterpret_cast<HMODULE>(library);
138+
FARPROC a = GetProcAddress (module, symbol);
139+
address = reinterpret_cast<void*>(a);
140+
141+
#else // ndef WINDOWS
142+
143+
address = dlsym (library, symbol);
144+
145+
#endif // ndef WINDOWS
146+
147+
if (address == nullptr) {
148+
_set_error_to_last_error (error);
149+
}
150+
151+
return address;
152+
}
153+
154+
int
155+
java_interop_close_library (void* library, char **error)
156+
{
157+
_free_error (error);
158+
if (library == nullptr) {
159+
_set_error (error, "library=nullptr");
160+
return JAVA_INTEROP_LIBRARY_INVALID_PARAM;
161+
}
162+
163+
int r = 0;
164+
165+
#ifdef WINDOWS
166+
HMODULE h = reinterpret_cast<HMODULE>(library);
167+
BOOL v = FreeLibrary (h);
168+
if (!v) {
169+
r = JAVA_INTEROP_LIBRARY_CLOSE_FAILED;
170+
}
171+
#else // ndef WINDOWS
172+
r = dlclose (library);
173+
if (r != 0) {
174+
r = JAVA_INTEROP_LIBRARY_CLOSE_FAILED;
175+
}
176+
#endif // ndef WINDOWS
177+
178+
if (r != 0) {
179+
_set_error_to_last_error (error);
180+
}
181+
182+
return r;
183+
}

‎src/java-interop/java-interop-dlfcn.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#ifndef INC_JAVA_INTEROP_DLFCN_H
2+
#define INC_JAVA_INTEROP_DLFCN_H
3+
4+
#include "java-interop.h"
5+
6+
JAVA_INTEROP_BEGIN_DECLS
7+
8+
// Possible error codes from java_interop_close_library
9+
constexpr int JAVA_INTEROP_LIBRARY_FAILED = -1000;
10+
constexpr int JAVA_INTEROP_LIBRARY_CLOSE_FAILED = JAVA_INTEROP_LIBRARY_FAILED-1;
11+
constexpr int JAVA_INTEROP_LIBRARY_INVALID_PARAM = JAVA_INTEROP_LIBRARY_FAILED-2;
12+
13+
14+
JAVA_INTEROP_API void* java_interop_load_library (const char *path, unsigned int flags, char **error);
15+
JAVA_INTEROP_API void* java_interop_get_symbol_address (void* library, const char *symbol, char **error);
16+
JAVA_INTEROP_API int java_interop_close_library (void* library, char **error);
17+
18+
JAVA_INTEROP_END_DECLS
19+
20+
#endif /* INC_JAVA_INTEROP_DLFCN_H */

‎src/java-interop/java-interop-gc-bridge-mono.cc‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
#include "logger.h"
2525
#endif /* !defined (ANDROID) */
2626

27-
#include <dlfcn.h>
28-
2927
#if defined (ANDROID) || defined (XAMARIN_ANDROID_DYLIB_MONO)
3028
using namespace xamarin::android;
3129
#endif
@@ -76,23 +74,6 @@ struct JavaInteropGCBridge {
7674
int gref_cleanup, lref_cleanup;
7775
};
7876

79-
typedef char* (*MonoThreadGetNameUtf8)(MonoThread*);
80-
typedef int32_t (*MonoThreadGetManagedId)(MonoThread*);
81-
82-
static MonoThreadGetManagedId _mono_thread_get_managed_id;
83-
static MonoThreadGetNameUtf8 _mono_thread_get_name_utf8;
84-
85-
static void
86-
lookup_optional_mono_thread_functions (void)
87-
{
88-
void *h = dlopen (NULL, RTLD_LAZY);
89-
if (!h)
90-
return;
91-
92-
_mono_thread_get_managed_id = reinterpret_cast<MonoThreadGetManagedId>(dlsym (h, "mono_thread_get_managed_id"));
93-
_mono_thread_get_name_utf8 = reinterpret_cast<MonoThreadGetNameUtf8>(dlsym (h, "mono_thread_get_name_utf8"));
94-
}
95-
9677
static jobject
9778
lref_to_gref (JNIEnv *env, jobject lref)
9879
{
@@ -275,8 +256,6 @@ java_interop_gc_bridge_new (JavaVM *jvm)
275256
}
276257
#endif /* defined (XAMARIN_ANDROID_DYLIB_MONO) */
277258

278-
lookup_optional_mono_thread_functions ();
279-
280259
JavaInteropGCBridge bridge = {0};
281260

282261
bridge.jvm = jvm;
@@ -1237,26 +1216,15 @@ gc_is_bridge_object (MonoObject *object)
12371216
static char *
12381217
get_thread_name (void)
12391218
{
1240-
if (_mono_thread_get_name_utf8) {
1241-
MonoThread *thread = mono_thread_current ();
1242-
return _mono_thread_get_name_utf8 (thread);
1243-
}
1244-
return strdup ("finalizer");
1219+
MonoThread *thread = mono_thread_current ();
1220+
return mono_thread_get_name_utf8 (thread);
12451221
}
12461222

12471223
static int64_t
12481224
get_thread_id (void)
12491225
{
1250-
if (_mono_thread_get_managed_id) {
1251-
MonoThread *thread = mono_thread_current ();
1252-
return _mono_thread_get_managed_id (thread);
1253-
}
1254-
#if __linux__
1255-
int64_t tid = (int64_t)((pid_t)syscall(SYS_gettid));
1256-
#else
1257-
int64_t tid = (int64_t) pthread_self ();
1258-
#endif
1259-
return tid;
1226+
MonoThread *thread = mono_thread_current ();
1227+
return mono_thread_get_managed_id (thread);
12601228
}
12611229

12621230
static void

‎src/java-interop/java-interop-gc-bridge.h‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,41 +19,41 @@ struct JavaInterop_System_RuntimeTypeHandle {
1919
void *value;
2020
};
2121

22-
MONO_API JavaInteropGCBridge *java_interop_gc_bridge_get_current (void);
23-
MONO_API int java_interop_gc_bridge_set_current_once (JavaInteropGCBridge *bridge);
22+
JAVA_INTEROP_API JavaInteropGCBridge *java_interop_gc_bridge_get_current (void);
23+
JAVA_INTEROP_API int java_interop_gc_bridge_set_current_once (JavaInteropGCBridge *bridge);
2424

25-
MONO_API JavaInteropGCBridge *java_interop_gc_bridge_new (JavaVM *jvm);
26-
MONO_API int java_interop_gc_bridge_free (JavaInteropGCBridge *bridge);
25+
JAVA_INTEROP_API JavaInteropGCBridge *java_interop_gc_bridge_new (JavaVM *jvm);
26+
JAVA_INTEROP_API int java_interop_gc_bridge_free (JavaInteropGCBridge *bridge);
2727

28-
MONO_API int java_interop_gc_bridge_register_hooks (JavaInteropGCBridge *bridge, int weak_ref_kind);
29-
MONO_API int java_interop_gc_bridge_wait_for_bridge_processing (JavaInteropGCBridge *bridge);
28+
JAVA_INTEROP_API int java_interop_gc_bridge_register_hooks (JavaInteropGCBridge *bridge, int weak_ref_kind);
29+
JAVA_INTEROP_API int java_interop_gc_bridge_wait_for_bridge_processing (JavaInteropGCBridge *bridge);
3030

31-
MONO_API int java_interop_gc_bridge_add_current_app_domain (JavaInteropGCBridge *bridge);
32-
MONO_API int java_interop_gc_bridge_remove_current_app_domain (JavaInteropGCBridge *bridge);
31+
JAVA_INTEROP_API int java_interop_gc_bridge_add_current_app_domain (JavaInteropGCBridge *bridge);
32+
JAVA_INTEROP_API int java_interop_gc_bridge_remove_current_app_domain (JavaInteropGCBridge *bridge);
3333

34-
MONO_API int java_interop_gc_bridge_set_bridge_processing_field (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle, char *field_name);
35-
MONO_API int java_interop_gc_bridge_register_bridgeable_type (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle);
36-
MONO_API int java_interop_gc_bridge_enable (JavaInteropGCBridge *bridge, int enable);
34+
JAVA_INTEROP_API int java_interop_gc_bridge_set_bridge_processing_field (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle, char *field_name);
35+
JAVA_INTEROP_API int java_interop_gc_bridge_register_bridgeable_type (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle);
36+
JAVA_INTEROP_API int java_interop_gc_bridge_enable (JavaInteropGCBridge *bridge, int enable);
3737

38-
MONO_API int java_interop_gc_bridge_get_gref_count (JavaInteropGCBridge *bridge);
39-
MONO_API int java_interop_gc_bridge_get_weak_gref_count (JavaInteropGCBridge *bridge);
38+
JAVA_INTEROP_API int java_interop_gc_bridge_get_gref_count (JavaInteropGCBridge *bridge);
39+
JAVA_INTEROP_API int java_interop_gc_bridge_get_weak_gref_count (JavaInteropGCBridge *bridge);
4040

41-
MONO_API int java_interop_gc_bridge_lref_set_log_file (JavaInteropGCBridge *bridge, const char *gref_log_file);
42-
MONO_API FILE* java_interop_gc_bridge_lref_get_log_file (JavaInteropGCBridge *bridge);
43-
MONO_API int java_interop_gc_bridge_lref_set_log_level (JavaInteropGCBridge *bridge, int level);
44-
MONO_API void java_interop_gc_bridge_lref_log_message (JavaInteropGCBridge *bridge, int level, const char *message);
45-
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);
46-
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);
41+
JAVA_INTEROP_API int java_interop_gc_bridge_lref_set_log_file (JavaInteropGCBridge *bridge, const char *gref_log_file);
42+
JAVA_INTEROP_API FILE* java_interop_gc_bridge_lref_get_log_file (JavaInteropGCBridge *bridge);
43+
JAVA_INTEROP_API int java_interop_gc_bridge_lref_set_log_level (JavaInteropGCBridge *bridge, int level);
44+
JAVA_INTEROP_API void java_interop_gc_bridge_lref_log_message (JavaInteropGCBridge *bridge, int level, const char *message);
45+
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);
46+
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);
4747

48-
MONO_API int java_interop_gc_bridge_gref_set_log_file (JavaInteropGCBridge *bridge, const char *gref_log_file);
49-
MONO_API FILE* java_interop_gc_bridge_gref_get_log_file (JavaInteropGCBridge *bridge);
50-
MONO_API int java_interop_gc_bridge_gref_set_log_level (JavaInteropGCBridge *bridge, int level);
51-
MONO_API void java_interop_gc_bridge_gref_log_message (JavaInteropGCBridge *bridge, int level, const char *message);
52-
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);
53-
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);
48+
JAVA_INTEROP_API int java_interop_gc_bridge_gref_set_log_file (JavaInteropGCBridge *bridge, const char *gref_log_file);
49+
JAVA_INTEROP_API FILE* java_interop_gc_bridge_gref_get_log_file (JavaInteropGCBridge *bridge);
50+
JAVA_INTEROP_API int java_interop_gc_bridge_gref_set_log_level (JavaInteropGCBridge *bridge, int level);
51+
JAVA_INTEROP_API void java_interop_gc_bridge_gref_log_message (JavaInteropGCBridge *bridge, int level, const char *message);
52+
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);
53+
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);
5454

55-
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);
56-
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);
55+
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);
56+
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);
5757

5858
JAVA_INTEROP_END_DECLS
5959

0 commit comments

Comments
 (0)