NativeBridge: Tighten security on libnativebridge
Do not allow arbitrary paths for the native bridge - only allow
simple names.
Do not allow re-setup of the native bridge.
Bug: 16404669
(cherry picked from commit cd2ef4c1af69727231b84ebc82864c170ff0e8ad)
Change-Id: Ie22de356d2307fe2758f9094a85d44e61a4098a1
diff --git a/libnativebridge/native_bridge.cc b/libnativebridge/native_bridge.cc
index ad4ee73..2205f45 100644
--- a/libnativebridge/native_bridge.cc
+++ b/libnativebridge/native_bridge.cc
@@ -16,6 +16,7 @@
#include "nativebridge/native_bridge.h"
+#include <cutils/log.h>
#include <dlfcn.h>
#include <stdio.h>
#include "utils/Mutex.h"
@@ -28,27 +29,92 @@
// The symbol name exposed by native-bridge with the type of NativeBridgeCallbacks.
static constexpr const char* kNativeBridgeInterfaceSymbol = "NativeBridgeItf";
-// The path of the library we are supposed to load.
-static const char* native_bridge_library_path = nullptr;
+// The filename of the library we are supposed to load.
+static const char* native_bridge_library_filename = nullptr;
// Whether a native bridge is available (loaded and ready).
static bool available = false;
// Whether we have already initialized (or tried to).
static bool initialized = false;
+// Whether we had an error at some point.
+static bool had_error = false;
static NativeBridgeCallbacks* callbacks = nullptr;
static const NativeBridgeRuntimeCallbacks* runtime_callbacks = nullptr;
-void SetupNativeBridge(const char* nb_library_path,
+// Characters allowed in a native bridge filename. The first character must
+// be in [a-zA-Z] (expected 'l' for "libx"). The rest must be in [a-zA-Z0-9._-].
+static bool CharacterAllowed(char c, bool first) {
+ if (first) {
+ return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z');
+ } else {
+ return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') || ('0' <= c && c <= '9') ||
+ (c == '.') || (c == '_') || (c == '-');
+ }
+}
+
+// We only allow simple names for the library. It is supposed to be a file in
+// /system/lib or /vendor/lib. Only allow a small range of characters, that is
+// names consisting of [a-zA-Z0-9._-] and starting with [a-zA-Z].
+bool NativeBridgeNameAcceptable(const char* nb_library_filename) {
+ const char* ptr = nb_library_filename;
+ if (*ptr == 0) {
+ // Emptry string. Allowed, means no native bridge.
+ return true;
+ } else {
+ // First character must be [a-zA-Z].
+ if (!CharacterAllowed(*ptr, true)) {
+ // Found an invalid fist character, don't accept.
+ ALOGE("Native bridge library %s has been rejected for first character %c", nb_library_filename, *ptr);
+ return false;
+ } else {
+ // For the rest, be more liberal.
+ ptr++;
+ while (*ptr != 0) {
+ if (!CharacterAllowed(*ptr, false)) {
+ // Found an invalid character, don't accept.
+ ALOGE("Native bridge library %s has been rejected for %c", nb_library_filename, *ptr);
+ return false;
+ }
+ ptr++;
+ }
+ }
+ return true;
+ }
+}
+
+void SetupNativeBridge(const char* nb_library_filename,
const NativeBridgeRuntimeCallbacks* runtime_cbs) {
Mutex::Autolock auto_lock(native_bridge_lock);
- native_bridge_library_path = nb_library_path;
+ if (initialized || native_bridge_library_filename != nullptr) {
+ // Setup has been called before. Ignore this call.
+ ALOGW("Called SetupNativeBridge for an already set up native bridge.");
+ // Note: counts as an error, even though the bridge may be functional.
+ had_error = true;
+ return;
+ }
+
runtime_callbacks = runtime_cbs;
- if (native_bridge_library_path == nullptr) {
- initialized = true;
+ if (nb_library_filename == nullptr) {
available = false;
+ initialized = true;
+ } else {
+ // Check whether it's an empty string.
+ if (*nb_library_filename == 0) {
+ available = false;
+ initialized = true;
+ } else if (!NativeBridgeNameAcceptable(nb_library_filename)) {
+ available = false;
+ initialized = true;
+ had_error = true;
+ }
+
+ if (!initialized) {
+ // Didn't find a name error or empty string, assign it.
+ native_bridge_library_filename = nb_library_filename;
+ }
}
}
@@ -62,7 +128,15 @@
available = false;
- void* handle = dlopen(native_bridge_library_path, RTLD_LAZY);
+ if (native_bridge_library_filename == nullptr) {
+ // Called initialize without setup. dlopen has special semantics for nullptr input.
+ // So just call it a day here. This counts as an error.
+ initialized = true;
+ had_error = true;
+ return false;
+ }
+
+ void* handle = dlopen(native_bridge_library_filename, RTLD_LAZY);
if (handle != nullptr) {
callbacks = reinterpret_cast<NativeBridgeCallbacks*>(dlsym(handle,
kNativeBridgeInterfaceSymbol));
@@ -72,8 +146,13 @@
}
if (!available) {
+ // If we fail initialization, this counts as an error.
+ had_error = true;
dlclose(handle);
}
+ } else {
+ // Being unable to open the library counts as an error.
+ had_error = true;
}
initialized = true;
@@ -81,6 +160,14 @@
return available;
}
+bool NativeBridgeError() {
+ return had_error;
+}
+
+bool NativeBridgeAvailable() {
+ return NativeBridgeInitialize();
+}
+
void* NativeBridgeLoadLibrary(const char* libpath, int flag) {
if (NativeBridgeInitialize()) {
return callbacks->loadLibrary(libpath, flag);