diff options
author | Treehugger Robot <android-test-infra-autosubmit@system.gserviceaccount.com> | 2023-09-25 20:41:33 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2023-09-25 20:41:33 +0000 |
commit | e40898a5b1d0d6d38c9d5256b9e968def5226c8b (patch) | |
tree | 292df4e7e44d7673e5601199479b89fcdb8a82b6 | |
parent | 4515bc009666daed8989c7e13d32ffdef2096193 (diff) | |
parent | dc1e927c68b24b333df3d2c04ee7e4b0f26060b7 (diff) | |
download | libnativehelper-e40898a5b1d0d6d38c9d5256b9e968def5226c8b.tar.gz |
Merge "Fix the use-after-free issue with CREATE_UTF_OR_RETURN." into main
-rw-r--r-- | header_only_include/nativehelper/utils.h | 22 | ||||
-rw-r--r-- | tests_mts/jni/libnativehelper_test.cpp | 40 |
2 files changed, 53 insertions, 9 deletions
diff --git a/header_only_include/nativehelper/utils.h b/header_only_include/nativehelper/utils.h index 30f239e..12f591b 100644 --- a/header_only_include/nativehelper/utils.h +++ b/header_only_include/nativehelper/utils.h @@ -35,8 +35,8 @@ namespace jnihelp { // Implementation details. DO NOT use directly. namespace internal { -[[maybe_unused]] static const char* get_c_str(const char* str) { return str; } -[[maybe_unused]] static const char* get_c_str(const std::string& str) { return str.c_str(); } +[[maybe_unused]] static const char* GetCStr(const char* str) { return str; } +[[maybe_unused]] static const char* GetCStr(const std::string& str) { return str.c_str(); } } // namespace internal @@ -103,12 +103,12 @@ class JniDefaultValue { #define GET_UTF_OR_RETURN_IMPL_(env, expr, ...) \ ({ \ - ScopedUtfChars tmp_scoped_utf_chars_(env, expr); \ - if (tmp_scoped_utf_chars_.c_str() == nullptr) { \ + ScopedUtfChars __or_return_scoped_utf_chars(env, expr); \ + if (__or_return_scoped_utf_chars.c_str() == nullptr) { \ /* Return with a pending exception from `ScopedUtfChars`. */ \ return __VA_ARGS__; \ } \ - std::move(tmp_scoped_utf_chars_); \ + std::move(__or_return_scoped_utf_chars); \ }) // Creates `ScopedLocalRef<jstring>` from a `const char*` or `std::string` expression using @@ -139,15 +139,19 @@ class JniDefaultValue { #define CREATE_UTF_OR_RETURN_IMPL_(env, expr, ...) \ ({ \ - const char* tmp_c_str_ = android::jnihelp::internal::get_c_str(expr); \ - ScopedLocalRef<jstring> tmp_local_ref_(env, env->NewStringUTF(tmp_c_str_)); \ + const char* __or_return_c_str; \ + ScopedLocalRef<jstring> __or_return_local_ref( \ + env, \ + env->NewStringUTF(__or_return_c_str = android::jnihelp::internal::GetCStr(expr))); \ + /* `*__or_return_c_str` may be freed here, but we only compare the pointer against \ + * nullptr. DO NOT DEREFERENCE `*__or_return_c_str` after this point. */ \ /* `NewStringUTF` returns nullptr when OOM or the input is nullptr, but only throws an \ * exception when OOM. */ \ - if (tmp_local_ref_ == nullptr && tmp_c_str_ != nullptr) { \ + if (__or_return_local_ref == nullptr && __or_return_c_str != nullptr) { \ /* Return with a pending exception from `NewStringUTF`. */ \ return __VA_ARGS__; \ } \ - std::move(tmp_local_ref_); \ + std::move(__or_return_local_ref); \ }) } // namespace jnihelp diff --git a/tests_mts/jni/libnativehelper_test.cpp b/tests_mts/jni/libnativehelper_test.cpp index 1063794..bd448f9 100644 --- a/tests_mts/jni/libnativehelper_test.cpp +++ b/tests_mts/jni/libnativehelper_test.cpp @@ -103,6 +103,46 @@ TEST_F(LibnativehelperTest, CreateUtfOrReturn) { EXPECT_EQ(ret, 1); } +class MyString : public std::string { + public: + explicit MyString(const char* c_str) : std::string(c_str) {} + + // Force clear the string to catch use-after-free issues. + ~MyString() { clear(); } +}; + +// `expr` creates a temporary object and evaluates to it. +TEST_F(LibnativehelperTest, CreateUtfOrReturnExprEvaluatesToTemporary) { + std::unique_ptr<ScopedLocalRef<jstring>> result; + + jint ret = [&](JNIEnv* env) -> jint { + ScopedLocalRef<jstring> j_str = CREATE_UTF_OR_RETURN(env, MyString("foo")); + result.reset(new ScopedLocalRef<jstring>(std::move(j_str))); + return 1; + }(mEnv); + + ScopedUtfChars str(mEnv, result->get()); + EXPECT_EQ(str.c_str(), std::string_view("foo")); + EXPECT_FALSE(mEnv->ExceptionCheck()); + EXPECT_EQ(ret, 1); +} + +// `expr` creates a temporary object and evaluates to something else backed by it. +TEST_F(LibnativehelperTest, CreateUtfOrReturnExprEvaluatesToValueBackedByTemporary) { + std::unique_ptr<ScopedLocalRef<jstring>> result; + + jint ret = [&](JNIEnv* env) -> jint { + ScopedLocalRef<jstring> j_str = CREATE_UTF_OR_RETURN(env, MyString("foo").c_str()); + result.reset(new ScopedLocalRef<jstring>(std::move(j_str))); + return 1; + }(mEnv); + + ScopedUtfChars str(mEnv, result->get()); + EXPECT_EQ(str.c_str(), std::string_view("foo")); + EXPECT_FALSE(mEnv->ExceptionCheck()); + EXPECT_EQ(ret, 1); +} + TEST_F(LibnativehelperTest, CreateUtfOrReturnVoid) { std::unique_ptr<ScopedLocalRef<jstring>> result; |