From 5019faa3a6bdde33785ece0537e8b865ea7cf938 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 19 Aug 2014 16:57:38 -0700 Subject: NativeHelper: Do not allow arbitrary library strings in user builds On device, only allow "libart.so" in non-debuggable (user) builds. Bug: 16404669 Change-Id: Ie163c04ce40c82698dcc98ced651dafef094d8b5 --- Android.mk | 4 +- JniInvocation.cpp | 28 ++++++- include/nativehelper/JniInvocation.h | 3 + tests/Android.mk | 33 ++++++++ tests/JniInvocation_test.cpp | 144 +++++++++++++++++++++++++++++++++++ 5 files changed, 208 insertions(+), 4 deletions(-) create mode 100644 tests/Android.mk create mode 100644 tests/JniInvocation_test.cpp diff --git a/Android.mk b/Android.mk index 29b851d..ef33104 100644 --- a/Android.mk +++ b/Android.mk @@ -32,7 +32,7 @@ LOCAL_SRC_FILES := \ LOCAL_SHARED_LIBRARIES := liblog LOCAL_MODULE_TAGS := optional LOCAL_MODULE := libnativehelper -LOCAL_CFLAGS := -Werror +LOCAL_CFLAGS := -Werror -fvisibility=protected LOCAL_C_INCLUDES := external/stlport/stlport bionic/ bionic/libstdc++/include libcore/include LOCAL_SHARED_LIBRARIES += libcutils libstlport libdl LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk @@ -71,7 +71,7 @@ LOCAL_MODULE_TAGS := optional LOCAL_SRC_FILES := \ $(local_src_files) \ JniInvocation.cpp -LOCAL_CFLAGS := -Werror +LOCAL_CFLAGS := -Werror -fvisibility=protected LOCAL_C_INCLUDES := libcore/include LOCAL_SHARED_LIBRARIES := liblog LOCAL_LDFLAGS := -ldl diff --git a/JniInvocation.cpp b/JniInvocation.cpp index 1764693..f4dd24e 100644 --- a/JniInvocation.cpp +++ b/JniInvocation.cpp @@ -50,13 +50,31 @@ JniInvocation::~JniInvocation() { #ifdef HAVE_ANDROID_OS static const char* kLibrarySystemProperty = "persist.sys.dalvik.vm.lib.2"; +static const char* kDebuggableSystemProperty = "ro.debuggable"; +static const char* kDebuggableFallback = "0"; // Not debuggable. #endif static const char* kLibraryFallback = "libart.so"; -bool JniInvocation::Init(const char* library) { +const char* JniInvocation::GetLibrary(const char* library) { #ifdef HAVE_ANDROID_OS char default_library[PROPERTY_VALUE_MAX]; - property_get(kLibrarySystemProperty, default_library, kLibraryFallback); + + char debuggable[PROPERTY_VALUE_MAX]; + property_get(kDebuggableSystemProperty, debuggable, kDebuggableFallback); + + if (strcmp(debuggable, "1") != 0) { + // Not a debuggable build. + // Do not allow arbitrary library. Ignore the library parameter. This + // will also ignore the default library, but initialize to empty string + // for cleanliness. + library = kLibraryFallback; + default_library[0] = 0; + } else { + // Debuggable build. + // Accept the library parameter. For the case it is NULL, load the default + // library from the system property. + property_get(kLibrarySystemProperty, default_library, kLibraryFallback); + } #else const char* default_library = kLibraryFallback; #endif @@ -64,6 +82,12 @@ bool JniInvocation::Init(const char* library) { library = default_library; } + return library; +} + +bool JniInvocation::Init(const char* library) { + library = GetLibrary(library); + handle_ = dlopen(library, RTLD_NOW); if (handle_ == NULL) { if (strcmp(library, kLibraryFallback) == 0) { diff --git a/include/nativehelper/JniInvocation.h b/include/nativehelper/JniInvocation.h index 9876d8d..b5198ff 100644 --- a/include/nativehelper/JniInvocation.h +++ b/include/nativehelper/JniInvocation.h @@ -38,6 +38,9 @@ class JniInvocation { // persist.sys.dalvik.vm.lib. bool Init(const char* library); + // Exposes which library is actually loaded from the given name. + static const char* GetLibrary(const char* library); + private: bool FindSymbol(void** pointer, const char* symbol); diff --git a/tests/Android.mk b/tests/Android.mk new file mode 100644 index 0000000..63e6f5c --- /dev/null +++ b/tests/Android.mk @@ -0,0 +1,33 @@ +# Build the unit tests. +LOCAL_PATH := $(call my-dir) +include $(CLEAR_VARS) + +# Target unit test. + +include $(CLEAR_VARS) +LOCAL_MODULE := JniInvocation_test +LOCAL_SRC_FILES := JniInvocation_test.cpp +LOCAL_SHARED_LIBRARIES := \ + libnativehelper + +include external/libcxx/libcxx.mk + +LOCAL_MULTILIB := both +LOCAL_MODULE_STEM_32 := $(LOCAL_MODULE)32 +LOCAL_MODULE_STEM_64 := $(LOCAL_MODULE)64 +include $(BUILD_NATIVE_TEST) + +# Host unit test. + +include $(CLEAR_VARS) +LOCAL_MODULE := JniInvocation_test +LOCAL_SRC_FILES := JniInvocation_test.cpp +LOCAL_SHARED_LIBRARIES := \ + libnativehelper + +include external/libcxx/libcxx.mk + +LOCAL_MULTILIB := both +LOCAL_MODULE_STEM_32 := $(LOCAL_MODULE)32 +LOCAL_MODULE_STEM_64 := $(LOCAL_MODULE)64 +include $(BUILD_HOST_NATIVE_TEST) diff --git a/tests/JniInvocation_test.cpp b/tests/JniInvocation_test.cpp new file mode 100644 index 0000000..6eb0267 --- /dev/null +++ b/tests/JniInvocation_test.cpp @@ -0,0 +1,144 @@ +/* + * 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. + */ + +#define LOG_TAG "NativeBridge_test" + +#include +#include + + +#include "string.h" + +#if defined(HAVE_ANDROID_OS) && defined(__BIONIC__) +#define HAVE_TEST_STUFF 1 +#else +#undef HAVE_TEST_STUFF +#endif + +#ifdef HAVE_TEST_STUFF + +// Ability to have fake local system properties. +#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ +#include + +extern void *__system_property_area__; + +struct LocalPropertyTestState { + LocalPropertyTestState() : valid(false) { + const char* ANDROID_DATA = getenv("ANDROID_DATA"); + char dir_template[PATH_MAX]; + snprintf(dir_template, sizeof(dir_template), "%s/local/tmp/prop-XXXXXX", ANDROID_DATA); + char* dirname = mkdtemp(dir_template); + if (!dirname) { + fprintf(stderr, "making temp file for test state failed (is %s writable?): %s", + dir_template, strerror(errno)); + return; + } + + old_pa = __system_property_area__; + __system_property_area__ = NULL; + + pa_dirname = dirname; + pa_filename = pa_dirname + "/__properties__"; + + __system_property_set_filename(pa_filename.c_str()); + __system_property_area_init(); + valid = true; + } + + ~LocalPropertyTestState() { + if (!valid) { + return; + } + + __system_property_area__ = old_pa; + + __system_property_set_filename(PROP_FILENAME); + unlink(pa_filename.c_str()); + rmdir(pa_dirname.c_str()); + } +public: + bool valid; +private: + std::string pa_dirname; + std::string pa_filename; + void *old_pa; +}; +#endif + +namespace android { + +class JNIInvocationTest : public testing::Test { +}; + +#ifdef HAVE_TEST_STUFF +static const char* kDebuggableSystemProperty = "ro.debuggable"; +static const char* kIsDebuggableValue = "1"; +static const char* kIsNotDebuggableValue = "0"; + +static const char* kLibrarySystemProperty = "persist.sys.dalvik.vm.lib.2"; +static const char* kTestNonNull = "libartd.so"; +static const char* kTestNonNull2 = "libartd2.so"; +static const char* kExpected = "libart.so"; +#endif + +TEST_F(JNIInvocationTest, Debuggable) { +#ifdef HAVE_TEST_STUFF + LocalPropertyTestState pa; + ASSERT_TRUE(pa.valid); + ASSERT_EQ(0, __system_property_add(kDebuggableSystemProperty, 13, kIsDebuggableValue, 1)); + ASSERT_EQ(0, __system_property_add(kLibrarySystemProperty, 27, kTestNonNull2, 11)); + + const char* result = JniInvocation::GetLibrary(NULL); + EXPECT_FALSE(result == NULL); + if (result != NULL) { + EXPECT_TRUE(strcmp(result, kTestNonNull2) == 0); + EXPECT_FALSE(strcmp(result, kExpected) == 0); + } + + result = JniInvocation::GetLibrary(kTestNonNull); + EXPECT_FALSE(result == NULL); + if (result != NULL) { + EXPECT_TRUE(strcmp(result, kTestNonNull) == 0); + EXPECT_FALSE(strcmp(result, kTestNonNull2) == 0); + } +#endif +} + +TEST_F(JNIInvocationTest, NonDebuggable) { +#ifdef HAVE_TEST_STUFF + LocalPropertyTestState pa; + ASSERT_TRUE(pa.valid); + ASSERT_EQ(0, __system_property_add(kDebuggableSystemProperty, 13, kIsNotDebuggableValue, 1)); + + const char* result = JniInvocation::GetLibrary(NULL); + EXPECT_FALSE(result == NULL); + if (result != NULL) { + EXPECT_TRUE(strcmp(result, kExpected) == 0); + EXPECT_FALSE(strcmp(result, kTestNonNull) == 0); + EXPECT_FALSE(strcmp(result, kTestNonNull2) == 0); + } + + result = JniInvocation::GetLibrary(kTestNonNull); + EXPECT_FALSE(result == NULL); + if (result != NULL) { + EXPECT_TRUE(strcmp(result, kExpected) == 0); + EXPECT_FALSE(strcmp(result, kTestNonNull) == 0); + } +#endif +} + +} // namespace android -- cgit v1.2.3