Merge "NativeBridge: Tighten security on libnativebridge" (e0677ccf) · Commits · e / os / android_system_core

include/nativebridge/native_bridge.h

+15 −0

Original line number	Diff line number	Diff line
		@@ -29,6 +29,10 @@ struct NativeBridgeRuntimeCallbacks;
		void SetupNativeBridge(const char* native_bridge_library_filename,
		const NativeBridgeRuntimeCallbacks* runtime_callbacks);

		// Check whether a native bridge is available (initialized). Requires a prior call to
		// SetupNativeBridge to make sense.
		bool NativeBridgeAvailable();

		// Load a shared library that is supported by the native bridge.
		void* NativeBridgeLoadLibrary(const char* libpath, int flag);

		@@ -38,6 +42,17 @@ void* NativeBridgeGetTrampoline(void* handle, const char* name, const char* shor
		// True if native library is valid and is for an ABI that is supported by native bridge.
		bool NativeBridgeIsSupported(const char* libpath);

		// Returns whether we have seen a native bridge error. This could happen because the library
		// was not found, rejected, could not be initialized and so on.
		//
		// This functionality is mainly for testing.
		bool NativeBridgeError();

		// Returns whether a given string is acceptable as a native bridge library filename.
		//
		// This functionality is exposed mainly for testing.
		bool NativeBridgeNameAcceptable(const char* native_bridge_library_filename);

		// Native bridge interfaces to runtime.
		struct NativeBridgeCallbacks {
		// Initialize native bridge. Native bridge's internal implementation must ensure MT safety and

libnativebridge/Android.mk

+4 −2

Original line number	Diff line number	Diff line
		@@ -10,10 +10,11 @@ include $(CLEAR_VARS)
		LOCAL_MODULE:= libnativebridge

		LOCAL_SRC_FILES:= $(NATIVE_BRIDGE_COMMON_SRC_FILES)
		LOCAL_SHARED_LIBRARIES := liblog
		LOCAL_CLANG := true
		LOCAL_CPP_EXTENSION := .cc
		LOCAL_CFLAGS := -Werror
		LOCAL_CPPFLAGS := -std=gnu++11
		LOCAL_CPPFLAGS := -std=gnu++11 -fvisibility=protected
		LOCAL_LDFLAGS := -ldl
		LOCAL_MULTILIB := both

		@@ -26,10 +27,11 @@ include $(CLEAR_VARS)
		LOCAL_MODULE:= libnativebridge

		LOCAL_SRC_FILES:= $(NATIVE_BRIDGE_COMMON_SRC_FILES)
		LOCAL_SHARED_LIBRARIES := liblog
		LOCAL_CLANG := true
		LOCAL_CPP_EXTENSION := .cc
		LOCAL_CFLAGS := -Werror
		LOCAL_CPPFLAGS := -std=gnu++11
		LOCAL_CPPFLAGS := -std=gnu++11 -fvisibility=protected
		LOCAL_LDFLAGS := -ldl
		LOCAL_MULTILIB := both

libnativebridge/native_bridge.cc

+94 −7

Original line number	Diff line number	Diff line
		@@ -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 @@ static Mutex native_bridge_lock("native bridge lock");
		// 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) {
		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 @@ static bool NativeBridgeInitialize() {

		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 @@ static bool NativeBridgeInitialize() {
		}

		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 @@ static bool NativeBridgeInitialize() {
		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);

libnativebridge/tests/Android.mk

0 → 100644

+33 −0

Original line number	Diff line number	Diff line
		# Build the unit tests.
		LOCAL_PATH := $(call my-dir)
		include $(CLEAR_VARS)

		# Build the unit tests.
		test_src_files := \
		InvalidCharsNativeBridge_test.cpp \
		ReSetupNativeBridge_test.cpp \
		UnavailableNativeBridge_test.cpp \
		ValidNameNativeBridge_test.cpp

		shared_libraries := \
		libnativebridge

		$(foreach file,$(test_src_files), \
		$(eval include $(CLEAR_VARS)) \
		$(eval LOCAL_CLANG := true) \
		$(eval LOCAL_CPPFLAGS := -std=gnu++11) \
		$(eval LOCAL_SHARED_LIBRARIES := $(shared_libraries)) \
		$(eval LOCAL_SRC_FILES := $(file)) \
		$(eval LOCAL_MODULE := $(notdir $(file:%.cpp=%))) \
		$(eval include $(BUILD_NATIVE_TEST)) \
		)

		$(foreach file,$(test_src_files), \
		$(eval include $(CLEAR_VARS)) \
		$(eval LOCAL_CLANG := true) \
		$(eval LOCAL_CPPFLAGS := -std=gnu++11) \
		$(eval LOCAL_SHARED_LIBRARIES := $(shared_libraries)) \
		$(eval LOCAL_SRC_FILES := $(file)) \
		$(eval LOCAL_MODULE := $(notdir $(file:%.cpp=%))) \
		$(eval include $(BUILD_HOST_NATIVE_TEST)) \
		)

libnativebridge/tests/InvalidCharsNativeBridge_test.cpp

0 → 100644

+40 −0

Original line number	Diff line number	Diff line
		/*
		* Copyright (C) 2014 The Android Open Source Project
		*
		* Licensed under the Apache License, Version 2.0 (the "License");
		* you may not use this file except in compliance with the License.
		* You may obtain a copy of the License at
		*
		* http://www.apache.org/licenses/LICENSE-2.0
		*
		* Unless required by applicable law or agreed to in writing, software
		* distributed under the License is distributed on an "AS IS" BASIS,
		* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
		* See the License for the specific language governing permissions and
		* limitations under the License.
		*/

		#include "NativeBridgeTest.h"

		namespace android {

		static const char* kTestName = "../librandom$@-bridge_not.existing.so";

		TEST_F(NativeBridgeTest, InvalidChars) {
		// Do one test actually calling setup.
		EXPECT_EQ(false, NativeBridgeError());
		SetupNativeBridge(kTestName, nullptr);
		// This should lead to an error for invalid characters.
		EXPECT_EQ(true, NativeBridgeError());

		// Further tests need to use NativeBridgeNameAcceptable, as the error
		// state can't be changed back.
		EXPECT_EQ(false, NativeBridgeNameAcceptable("."));
		EXPECT_EQ(false, NativeBridgeNameAcceptable(".."));
		EXPECT_EQ(false, NativeBridgeNameAcceptable("_"));
		EXPECT_EQ(false, NativeBridgeNameAcceptable("-"));
		EXPECT_EQ(false, NativeBridgeNameAcceptable("lib@.so"));
		EXPECT_EQ(false, NativeBridgeNameAcceptable("lib$.so"));
		}

		} // namespace android