Skip to content

Commit 3de6d66

Browse files
committed
TESTING TESTING TESTING TESTING TESTING DO NOT MERGE DO NOT MERGE DO NOT MERGE Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578 Context: dotnet/java-interop#691 Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide For improved security, the guidance is that when loading `.dll` files on Windows, the default [`LoadLibrary()`][0] behavior is to be *avoided*, as the [Dynamic-Link Library Search Order][1] includes the "current working directory" and directories listed in `%PATH%`, both of which are under control of an "attacker" and not the application. Instead, [`LoadLibraryEx()`][2] should be used, providing a set of `LOAD_LIBRARY_SEARCH_*` values which constrain the set of directories that will be searched for the library (if a full path is not used) and/or the library's dependencies. Historically, Xamarin.Android hasn't directly called any `LoadLibrary()` function. Instead, xamarin-android called [**dlopen**(3)][3], then used dlfcn-win32/dlfcn-win32@ef7e412d to implement `dlopen()` in terms of `LoadLibraryEx()`. Unfortunately, dlfcn-win32/dlfcn-win32@ef7e412d calls `LoadLibraryEx()` with `LOAD_WITH_ALTERED_SEARCH_PATH`, which while a security improvement, does not go far enough for our internal requirements. Migrate away from dlfcn-win32 and instead use the new `java-interop-dlfcn.h` family of functions added in TODO(dotnet/java-interop#691). These new functions appropriately use `LoadLibraryEx()` with a constrained set of `LOAD_LIBRARY_SEARCH_*` flags. [0]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibrarya [1]: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order [2]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa [3]: https://linux.die.net/man/3/dlopen
1 parent 53ec311 commit 3de6d66

File tree

10 files changed

+44
-64
lines changed

10 files changed

+44
-64
lines changed

.gitmodules

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,10 @@
66
path = external/debugger-libs
77
url = https://github.com/mono/debugger-libs
88
branch = master
9-
[submodule "external/dlfcn-win32"]
10-
path = external/dlfcn-win32
11-
url = https://github.com/dlfcn-win32/dlfcn-win32.git
12-
branch = v1.1.1
139
[submodule "external/Java.Interop"]
1410
path = external/Java.Interop
15-
url = https://github.com/xamarin/java.interop.git
16-
branch = master
11+
url = https://github.com/jonpryor/java.interop.git
12+
branch = jonp-loadlib-search-order
1713
[submodule "external/lz4"]
1814
path = external/lz4
1915
url = https://github.com/lz4/lz4.git

build-tools/xaprepare/xaprepare/Steps/Step_BuildMingwDependencies.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ namespace Xamarin.Android.Prepare
88
class Step_BuildMingwDependencies : Step
99
{
1010
static readonly SortedDictionary <string, (string description, string libraryName)> dependencies = new SortedDictionary <string, (string description, string libraryName)> (StringComparer.Ordinal) {
11-
{ "dlfcn-win32", (description: "libdl for Windows", libraryName: "libdl.a") },
1211
{ "mman-win32", (description: "mmap for Windows", libraryName: "libmman.a") },
1312
};
1413

build-tools/xaprepare/xaprepare/ThirdPartyNotices/dlfcn.cs

Lines changed: 0 additions & 21 deletions
This file was deleted.

build-tools/xaprepare/xaprepare/xaprepare.csproj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,6 @@
157157
<Compile Include="Steps\Step_ThirdPartyNotices.cs" />
158158
<Compile Include="ThirdPartyNotices\aapt2.cs" />
159159
<Compile Include="ThirdPartyNotices\bundletool.cs" />
160-
<Compile Include="ThirdPartyNotices\dlfcn.cs" />
161160
<Compile Include="ThirdPartyNotices\Java.Interop.cs" />
162161
<Compile Include="ThirdPartyNotices\K4os.Compression.LZ4.cs" />
163162
<Compile Include="ThirdPartyNotices\lz4.cs" />

external/dlfcn-win32

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/monodroid/CMakeLists.txt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ else()
196196
if(WIN32 OR MINGW)
197197
message(STATUS "Win32 or MinGW")
198198
set(EXTRA_COMPILER_FLAGS "${EXTRA_COMPILER_FLAGS} -DWINDOWS -DNTDDI_VERSION=NTDDI_VISTA -D_WIN32_WINNT=_WIN32_WINNT_VISTA -fomit-frame-pointer")
199-
set(EXTRA_LINKER_FLAGS "${EXTRA_LINKER_FLAGS} -ldl -lmman -static -pthread -dynamic -lmincore -lmswsock -lwsock32 -lshlwapi -lpsapi -lwinmm")
199+
set(EXTRA_LINKER_FLAGS "${EXTRA_LINKER_FLAGS} -lmman -static -pthread -dynamic -lmincore -lmswsock -lwsock32 -lshlwapi -lpsapi -lwinmm")
200200

201201
if (MINGW_TARGET_32)
202202
set(ANDROID_ABI "host-mxe-Win32")
@@ -298,6 +298,7 @@ set(MONODROID_SOURCES
298298
${JAVA_INTEROP_SRC_PATH}/java-interop.cc
299299
${JAVA_INTEROP_SRC_PATH}/java-interop-mono.cc
300300
${JAVA_INTEROP_SRC_PATH}/java-interop-util.cc
301+
${JAVA_INTEROP_SRC_PATH}/java-interop-dlfcn.cc
301302
)
302303

303304
set(XA_INTERNAL_API_SOURCES
@@ -392,11 +393,13 @@ else()
392393
set(LINK_LIBS "-lmonosgen-2.0 -lz ${EXTRA_LINKER_FLAGS}")
393394
endif()
394395

395-
set(DEBUG_HELPER_LINK_LIBS "-ldl")
396+
if(NOT MINGW AND NOT WIN32)
397+
set(DEBUG_HELPER_LINK_LIBS "-ldl")
398+
endif()
396399
if(ENABLE_NDK)
397400
set(LINK_LIBS "${LINK_LIBS} -llog")
398401
set(DEBUG_HELPER_LINK_LIBS "${DEBUG_HELPER_LINK_LIBS} -llog")
399-
elseif(NOT ANDROID)
402+
elseif(NOT ANDROID AND NOT MINGW AND NOT WIN32)
400403
set(LINK_LIBS "-pthread ${LINK_LIBS} -ldl")
401404
endif()
402405

src/monodroid/jni/android-system.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
#include <errno.h>
55
#include <assert.h>
66
#include <ctype.h>
7-
#include <dlfcn.h>
87
#include <fcntl.h>
98

109
#ifdef ANDROID
@@ -27,6 +26,8 @@
2726
#include "jni-wrappers.hh"
2827
#include "xamarin-app.hh"
2928
#include "cpp-util.hh"
29+
#include "java-interop-dlfcn.h"
30+
#include "java-interop.h"
3031

3132
#if defined (DEBUG) || !defined (ANDROID)
3233
namespace xamarin::android::internal {
@@ -358,9 +359,11 @@ AndroidSystem::load_dso (const char *path, int dl_flags, bool skip_exists_check)
358359
return nullptr;
359360
}
360361

361-
void *handle = dlopen (path, dl_flags);
362+
char *error = nullptr;
363+
void *handle = java_interop_load_library (path, 0, &error);
362364
if (handle == nullptr && utils.should_log (LOG_ASSEMBLY))
363-
log_info_nocheck (LOG_ASSEMBLY, "Failed to load shared library '%s'. %s", path, dlerror ());
365+
log_info_nocheck (LOG_ASSEMBLY, "Failed to load shared library '%s'. %s", path, error);
366+
java_interop_free (error);
364367
return handle;
365368
}
366369

src/monodroid/jni/debug.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
#include <ctype.h>
2828
#include <assert.h>
2929
#include <limits.h>
30-
#include <dlfcn.h>
3130
#include <mono/metadata/mono-debug.h>
3231

3332
#ifdef ANDROID
@@ -42,6 +41,8 @@
4241
#include "globals.hh"
4342
#include "cpp-util.hh"
4443

44+
#include "java-interop-dlfcn.h"
45+
4546
using namespace xamarin::android;
4647

4748
//
@@ -79,15 +80,14 @@ Debug::monodroid_profiler_load (const char *libmono_path, const char *desc, cons
7980
}
8081
simple_pointer_guard<char[]> mname (mname_ptr);
8182

82-
int dlopen_flags = RTLD_LAZY;
8383
simple_pointer_guard<char[]> libname (utils.string_concat ("libmono-profiler-", mname.get (), ".so"));
8484
bool found = false;
85-
void *handle = androidSystem.load_dso_from_any_directories (libname, dlopen_flags);
85+
void *handle = androidSystem.load_dso_from_any_directories (libname, 0);
8686
found = load_profiler_from_handle (handle, desc, mname);
8787

8888
if (!found && libmono_path != nullptr) {
8989
simple_pointer_guard<char[]> full_path (utils.path_combine (libmono_path, libname));
90-
handle = androidSystem.load_dso (full_path, dlopen_flags, FALSE);
90+
handle = androidSystem.load_dso (full_path, 0, FALSE);
9191
found = load_profiler_from_handle (handle, desc, mname);
9292
}
9393

@@ -108,7 +108,7 @@ typedef void (*ProfilerInitializer) (const char*);
108108
bool
109109
Debug::load_profiler (void *handle, const char *desc, const char *symbol)
110110
{
111-
ProfilerInitializer func = reinterpret_cast<ProfilerInitializer> (dlsym (handle, symbol));
111+
ProfilerInitializer func = reinterpret_cast<ProfilerInitializer> (java_interop_get_symbol_address (handle, symbol, nullptr));
112112
log_warn (LOG_DEFAULT, "Looking for profiler init symbol '%s'? %p", symbol, func);
113113

114114
if (func != nullptr) {
@@ -129,7 +129,7 @@ Debug::load_profiler_from_handle (void *dso_handle, const char *desc, const char
129129

130130
if (result)
131131
return true;
132-
dlclose (dso_handle);
132+
java_interop_close_library (dso_handle, nullptr);
133133
return false;
134134
}
135135

src/monodroid/jni/monodroid-glue.cc

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@
1010
#include <errno.h>
1111
#include <limits.h>
1212

13+
#if defined (APPLE_OS_X)
1314
#include <dlfcn.h>
15+
#endif // def APPLE_OX_X
16+
1417
#include <fcntl.h>
1518
#include <unistd.h>
1619
#include <stdint.h>
@@ -82,6 +85,8 @@
8285

8386
#include "cpp-util.hh"
8487

88+
#include "java-interop-dlfcn.h"
89+
8590
using namespace xamarin::android;
8691
using namespace xamarin::android::internal;
8792

@@ -151,12 +156,11 @@ MonodroidRuntime::setup_bundled_app (const char *dso_name)
151156
if (!application_config.is_a_bundled_app)
152157
return;
153158

154-
static int dlopen_flags = RTLD_LAZY;
155159
void *libapp = nullptr;
156160

157161
if (androidSystem.is_embedded_dso_mode_enabled ()) {
158162
log_info (LOG_DEFAULT, "bundle app: embedded DSO mode");
159-
libapp = androidSystem.load_dso_from_any_directories (dso_name, dlopen_flags);
163+
libapp = androidSystem.load_dso_from_any_directories (dso_name, 0);
160164
} else {
161165
bool needs_free = false;
162166
log_info (LOG_DEFAULT, "bundle app: normal mode");
@@ -165,7 +169,7 @@ MonodroidRuntime::setup_bundled_app (const char *dso_name)
165169
if (bundle_path == nullptr)
166170
return;
167171
log_info (LOG_BUNDLE, "Attempting to load bundled app from %s", bundle_path);
168-
libapp = androidSystem.load_dso (bundle_path, dlopen_flags, true);
172+
libapp = androidSystem.load_dso (bundle_path, 0, true);
169173
if (needs_free)
170174
delete[] bundle_path;
171175
}
@@ -181,11 +185,11 @@ MonodroidRuntime::setup_bundled_app (const char *dso_name)
181185
}
182186
}
183187

184-
mono_mkbundle_initialize_mono_api = reinterpret_cast<mono_mkbundle_initialize_mono_api_ptr> (dlsym (libapp, "initialize_mono_api"));
188+
mono_mkbundle_initialize_mono_api = reinterpret_cast<mono_mkbundle_initialize_mono_api_ptr> (java_interop_get_symbol_address (libapp, "initialize_mono_api", nullptr));
185189
if (!mono_mkbundle_initialize_mono_api)
186190
log_error (LOG_BUNDLE, "Missing initialize_mono_api in the application");
187191

188-
mono_mkbundle_init = reinterpret_cast<mono_mkbundle_init_ptr> (dlsym (libapp, "mono_mkbundle_init"));
192+
mono_mkbundle_init = reinterpret_cast<mono_mkbundle_init_ptr> (java_interop_get_symbol_address (libapp, "mono_mkbundle_init", nullptr));
189193
if (!mono_mkbundle_init)
190194
log_error (LOG_BUNDLE, "Missing mono_mkbundle_init in the application");
191195
log_info (LOG_BUNDLE, "Bundled app loaded: %s", dso_name);
@@ -1067,13 +1071,7 @@ setup_gc_logging (void)
10671071
force_inline int
10681072
MonodroidRuntime::convert_dl_flags (int flags)
10691073
{
1070-
int lflags = flags & static_cast<int> (MONO_DL_LOCAL) ? 0: RTLD_GLOBAL;
1071-
1072-
if (flags & static_cast<int> (MONO_DL_LAZY))
1073-
lflags |= RTLD_LAZY;
1074-
else
1075-
lflags |= RTLD_NOW;
1076-
return lflags;
1074+
return 0;
10771075
}
10781076

10791077
force_inline void
@@ -1099,7 +1097,7 @@ MonodroidRuntime::init_internal_api_dso (void *handle)
10991097

11001098
std::lock_guard<std::mutex> lock (api_init_lock);
11011099
if (api_dso_handle != nullptr) {
1102-
auto api_shutdown = reinterpret_cast<external_api_shutdown_fn> (dlsym (api_dso_handle, MonoAndroidInternalCalls::SHUTDOWN_FUNCTION_NAME));
1100+
auto api_shutdown = reinterpret_cast<external_api_shutdown_fn> (java_interop_get_symbol_address (api_dso_handle, MonoAndroidInternalCalls::SHUTDOWN_FUNCTION_NAME, nullptr));
11031101
if (api_shutdown == nullptr) {
11041102
// We COULD ignore this situation, but if the function is missing it means we messed something up and thus
11051103
// it *is* a fatal error.
@@ -1111,7 +1109,7 @@ MonodroidRuntime::init_internal_api_dso (void *handle)
11111109

11121110
api_dso_handle = handle;
11131111
auto api = new MonoAndroidInternalCalls_Impl ();
1114-
auto api_init = reinterpret_cast<external_api_init_fn>(dlsym (handle, MonoAndroidInternalCalls::INIT_FUNCTION_NAME));
1112+
auto api_init = reinterpret_cast<external_api_init_fn>(java_interop_get_symbol_address (handle, MonoAndroidInternalCalls::INIT_FUNCTION_NAME, nullptr));
11151113
if (api_init == nullptr) {
11161114
log_fatal (LOG_DEFAULT, "Unable to initialize Internal API library, init function '%s' not found in the module", MonoAndroidInternalCalls::INIT_FUNCTION_NAME);
11171115
exit (FATAL_EXIT_MONO_MISSING_SYMBOLS);
@@ -1190,11 +1188,15 @@ void*
11901188
MonodroidRuntime::monodroid_dlsym (void *handle, const char *name, char **err, [[maybe_unused]] void *user_data)
11911189
{
11921190
void *s;
1191+
char *e = nullptr;
11931192

1194-
s = dlsym (handle, name);
1193+
s = java_interop_get_symbol_address (handle, name, &e);
11951194

11961195
if (!s && err) {
1197-
*err = utils.monodroid_strdup_printf ("Could not find symbol '%s'.", name);
1196+
*err = utils.monodroid_strdup_printf ("Could not find symbol '%s': %s", name, e);
1197+
}
1198+
if (e) {
1199+
java_interop_free (e);
11981200
}
11991201

12001202
return s;
@@ -1370,9 +1372,9 @@ MonodroidRuntime::disable_external_signal_handlers (void)
13701372
if (!androidSystem.is_mono_llvm_enabled ())
13711373
return;
13721374

1373-
void *llvm = androidSystem.load_dso ("libLLVM.so", RTLD_LAZY, TRUE);
1375+
void *llvm = androidSystem.load_dso ("libLLVM.so", 0, TRUE);
13741376
if (llvm) {
1375-
bool *disable_signals = reinterpret_cast<bool*> (dlsym (llvm, "_ZN4llvm23DisablePrettyStackTraceE"));
1377+
bool *disable_signals = reinterpret_cast<bool*> (java_interop_get_symbol_address (llvm, "_ZN4llvm23DisablePrettyStackTraceE", nullptr));
13761378
if (disable_signals) {
13771379
*disable_signals = true;
13781380
log_info (LOG_DEFAULT, "Disabled LLVM signal trapping");
@@ -1605,7 +1607,7 @@ MonodroidRuntime::Java_mono_android_Runtime_initInternal (JNIEnv *env, jclass kl
16051607
if (my_location != nullptr) {
16061608
simple_pointer_guard<char, false> dso_path (utils.path_combine (my_location, API_DSO_NAME));
16071609
log_info (LOG_DEFAULT, "Attempting to load %s", dso_path.get ());
1608-
api_dso_handle = dlopen (dso_path.get (), RTLD_NOW);
1610+
api_dso_handle = java_interop_load_library (dso_path.get (), 0, nullptr);
16091611
#if defined (APPLE_OS_X)
16101612
delete[] my_location;
16111613
#else // !defined(APPLE_OS_X)
@@ -1615,11 +1617,11 @@ MonodroidRuntime::Java_mono_android_Runtime_initInternal (JNIEnv *env, jclass kl
16151617

16161618
if (api_dso_handle == nullptr) {
16171619
log_info (LOG_DEFAULT, "Attempting to load %s with \"bare\" dlopen", API_DSO_NAME);
1618-
api_dso_handle = dlopen (API_DSO_NAME, RTLD_NOW);
1620+
api_dso_handle = java_interop_load_library (API_DSO_NAME, 0, nullptr);
16191621
}
16201622
#endif // defined(WINDOWS) || defined(APPLE_OS_X)
16211623
if (api_dso_handle == nullptr)
1622-
api_dso_handle = androidSystem.load_dso_from_any_directories (API_DSO_NAME, RTLD_NOW);
1624+
api_dso_handle = androidSystem.load_dso_from_any_directories (API_DSO_NAME, 0);
16231625
init_internal_api_dso (api_dso_handle);
16241626

16251627
mono_dl_fallback_register (monodroid_dlopen, monodroid_dlsym, nullptr, nullptr);

0 commit comments

Comments
 (0)