Skip to content

Commit 08ff4db

Browse files
jonpryorjonpryor
authored
[java-interop, Java.Interop] Securely load native libs (#691)
Fixes: #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)][3] on Windows, the following functions are added to support loading native libraries and resolving symbols from those native libraries: void* java_interop_lib_load (const char *path, unsigned int flags, char **error); void* java_interop_lib_symbol (void* library, const char *symbol, char **error); int java_interop_lib_close (void* library, char **error); (Previously, xamarin-android used the [dlfcn-win32/dlfcn-win32][4] library to implement **dlopen**(3), but dlfcn-win32/dlfcn-win32@ef7e412d calls `LoadLibraryEx()` with `LOAD_WITH_ALTERED_SEARCH_PATH`, which doesn't fulfill our internal requirements.) On Windows, `java_interop_lib_load()` will use [`LoadLibraryEx()`][5] 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_lib_load()`, 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://github.com/dlfcn-win32/dlfcn-win32 [5]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa
1 parent 09c6891 commit 08ff4db

File tree

13 files changed

+342
-145
lines changed

13 files changed

+342
-145
lines changed

‎src/Java.Interop/Properties/AssemblyInfo.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
using System.Reflection;
22
using System.Runtime.CompilerServices;
3+
using System.Runtime.InteropServices;
4+
5+
[assembly: DefaultDllImportSearchPathsAttribute (DllImportSearchPath.SafeDirectories | DllImportSearchPath.AssemblyDirectory)]
36

47
[assembly: AssemblyTitle ("Java.Interop")]
58
[assembly: AssemblyDescription ("")]

‎src/Java.Runtime.Environment/Properties/AssemblyInfo.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
using System.Runtime.InteropServices;
2+
3+
[assembly: DefaultDllImportSearchPathsAttribute (DllImportSearchPath.SafeDirectories | DllImportSearchPath.AssemblyDirectory)]

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

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

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#ifndef INC_JAVA_INTEROP_DLFCN_H
2+
#define INC_JAVA_INTEROP_DLFCN_H
3+
4+
#include "java-interop.h"
5+
6+
namespace microsoft::java_interop {
7+
8+
// Possible flags values for java_interop_lib_load
9+
constexpr unsigned int JAVA_INTEROP_LIB_LOAD_GLOBALLY = (1 << 0);
10+
constexpr unsigned int JAVA_INTEROP_LIB_LOAD_LOCALLY = (1 << 1);
11+
12+
13+
// Possible error codes from java_interop_lib_close
14+
constexpr int JAVA_INTEROP_LIB_FAILED = -1000;
15+
constexpr int JAVA_INTEROP_LIB_CLOSE_FAILED = JAVA_INTEROP_LIB_FAILED-1;
16+
constexpr int JAVA_INTEROP_LIB_INVALID_PARAM = JAVA_INTEROP_LIB_FAILED-2;
17+
18+
JAVA_INTEROP_BEGIN_DECLS
19+
20+
JAVA_INTEROP_API void* java_interop_lib_load (const char *path, unsigned int flags, char **error);
21+
JAVA_INTEROP_API void* java_interop_lib_symbol (void* library, const char *symbol, char **error);
22+
JAVA_INTEROP_API int java_interop_lib_close (void* library, char **error);
23+
24+
JAVA_INTEROP_END_DECLS
25+
26+
}
27+
28+
#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)