Harden hidden api checks.
Also walk past j.l.r in stackwalk for Hidden API. This is a fix for an
asan test failure (691-hiddenapi-proxy) introduced in https://r.android.com/1208005.
Instead of always walking past j.l.r during the stackwalk, this CL adds an
exception for j.l.r.Proxy.
Bug: 142365358
Test: art/test/testrunner/testrunner.py --target --64 -t674-hiddenapi
Test: art/test/testrunner/run_build_test_target.py -j110 art-asan
Change-Id: I98dd46d7070dd2dd4318398d2a5d2ae4ece94015
diff --git a/runtime/native/java_lang_Class.cc b/runtime/native/java_lang_Class.cc
index da87713..2c537c6 100644
--- a/runtime/native/java_lang_Class.cc
+++ b/runtime/native/java_lang_Class.cc
@@ -39,6 +39,7 @@
#include "mirror/object-inl.h"
#include "mirror/object_array-alloc-inl.h"
#include "mirror/object_array-inl.h"
+#include "mirror/proxy.h"
#include "mirror/string-alloc-inl.h"
#include "mirror/string-inl.h"
#include "native_util.h"
@@ -55,12 +56,17 @@
namespace art {
+// Should be the same as dalvik.system.VMRuntime.PREVENT_META_REFLECTION_BLACKLIST_ACCESS.
+// Corresponds to a bug id.
+static constexpr uint64_t kPreventMetaReflectionBlacklistAccess = 142365358;
+
// Walks the stack, finds the caller of this reflective call and returns
// a hiddenapi AccessContext formed from its declaring class.
static hiddenapi::AccessContext GetReflectionCaller(Thread* self)
REQUIRES_SHARED(Locks::mutator_lock_) {
- // Walk the stack and find the first frame not from java.lang.Class and not
- // from java.lang.invoke. This is very expensive. Save this till the last.
+ // Walk the stack and find the first frame not from java.lang.Class,
+ // java.lang.invoke or java.lang.reflect. This is very expensive.
+ // Save this till the last.
struct FirstExternalCallerVisitor : public StackVisitor {
explicit FirstExternalCallerVisitor(Thread* thread)
: StackVisitor(thread, nullptr, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
@@ -93,6 +99,16 @@
&& !m->IsClassInitializer()) {
return true;
}
+ // Check for classes in the java.lang.reflect package, except for java.lang.reflect.Proxy.
+ // java.lang.reflect.Proxy does its own hidden api checks (https://r.android.com/915496),
+ // and walking over this frame would cause a null pointer dereference
+ // (e.g. in 691-hiddenapi-proxy).
+ ObjPtr<mirror::Class> proxy_class = GetClassRoot<mirror::Proxy>();
+ if (declaring_class->IsInSamePackage(proxy_class) && declaring_class != proxy_class) {
+ if (Runtime::Current()->isChangeEnabled(kPreventMetaReflectionBlacklistAccess)) {
+ return true;
+ }
+ }
}
caller = m;
diff --git a/test/674-hiddenapi/hiddenapi.cc b/test/674-hiddenapi/hiddenapi.cc
index 413c707..742b6b3 100644
--- a/test/674-hiddenapi/hiddenapi.cc
+++ b/test/674-hiddenapi/hiddenapi.cc
@@ -27,6 +27,9 @@
namespace art {
namespace Test674HiddenApi {
+// Should be the same as dalvik.system.VMRuntime.PREVENT_META_REFLECTION_BLACKLIST_ACCESS
+static constexpr uint64_t kPreventMetaReflectionBlacklistAccess = 142365358;
+
std::vector<std::vector<std::unique_ptr<const DexFile>>> opened_dex_files;
extern "C" JNIEXPORT void JNICALL Java_Main_init(JNIEnv*, jclass) {
@@ -316,5 +319,18 @@
return static_cast<jint>(kAccHiddenapiBits);
}
+extern "C" JNIEXPORT void JNICALL Java_Reflection_setHiddenApiCheckHardening(JNIEnv*, jclass,
+ jboolean value) {
+ std::set<uint64_t> disabled_changes = Runtime::Current()->GetDisabledCompatChanges();
+ if (value == JNI_TRUE) {
+ // If hidden api check hardening is enabled, remove it from the set of disabled changes.
+ disabled_changes.erase(kPreventMetaReflectionBlacklistAccess);
+ } else {
+ // If hidden api check hardening is disabled, add it to the set of disabled changes.
+ disabled_changes.insert(kPreventMetaReflectionBlacklistAccess);
+ }
+ Runtime::Current()->SetDisabledCompatChanges(disabled_changes);
+}
+
} // namespace Test674HiddenApi
} // namespace art
diff --git a/test/674-hiddenapi/src-ex/ChildClass.java b/test/674-hiddenapi/src-ex/ChildClass.java
index f120bda..9295655 100644
--- a/test/674-hiddenapi/src-ex/ChildClass.java
+++ b/test/674-hiddenapi/src-ex/ChildClass.java
@@ -105,6 +105,13 @@
boolean isSameBoot = (isParentInBoot == isChildInBoot);
boolean isDebuggable = VMRuntime.getRuntime().isJavaDebuggable();
+ // For compat reasons, meta-reflection should still be usable by apps if hidden api check
+ // hardening is disabled (i.e. target SDK is Q or earlier). The only configuration where this
+ // workaround used to work is for ChildClass in the Application domain and ParentClass in the
+ // Platform domain, so only test that configuration with hidden api check hardening disabled.
+ boolean testHiddenApiCheckHardeningDisabled =
+ (childDomain == DexDomain.Application) && (parentDomain == DexDomain.Platform);
+
// Run meaningful combinations of access flags.
for (Hiddenness hiddenness : Hiddenness.values()) {
final Behaviour expected;
@@ -138,18 +145,19 @@
for (Class klass : new Class<?>[] { ParentClass.class, ParentInterface.class }) {
String baseName = visibility.name() + suffix;
checkField(klass, "field" + baseName, isStatic, visibility, expected,
- invokesMemberCallback);
+ invokesMemberCallback, testHiddenApiCheckHardeningDisabled);
checkMethod(klass, "method" + baseName, isStatic, visibility, expected,
- invokesMemberCallback);
+ invokesMemberCallback, testHiddenApiCheckHardeningDisabled);
}
// Check whether one can use a class constructor.
- checkConstructor(ParentClass.class, visibility, hiddenness, expected);
+ checkConstructor(ParentClass.class, visibility, hiddenness, expected,
+ testHiddenApiCheckHardeningDisabled);
// Check whether one can use an interface default method.
String name = "method" + visibility.name() + "Default" + hiddenness.name();
checkMethod(ParentInterface.class, name, /*isStatic*/ false, visibility, expected,
- invokesMemberCallback);
+ invokesMemberCallback, testHiddenApiCheckHardeningDisabled);
}
// Test whether static linking succeeds.
@@ -212,7 +220,8 @@
}
private static void checkField(Class<?> klass, String name, boolean isStatic,
- Visibility visibility, Behaviour behaviour, boolean invokesMemberCallback) throws Exception {
+ Visibility visibility, Behaviour behaviour, boolean invokesMemberCallback,
+ boolean testHiddenApiCheckHardeningDisabled) throws Exception {
boolean isPublic = (visibility == Visibility.Public);
boolean canDiscover = (behaviour != Behaviour.Denied);
@@ -277,6 +286,23 @@
canDiscover);
}
+ // Check for meta reflection.
+
+ // With hidden api check hardening enabled, only white and light greylisted fields should be
+ // discoverable.
+ if (Reflection.canDiscoverFieldWithMetaReflection(klass, name, true) != canDiscover) {
+ throwDiscoveryException(klass, name, false,
+ "Meta reflection with hidden api hardening enabled", canDiscover);
+ }
+
+ if (testHiddenApiCheckHardeningDisabled) {
+ // With hidden api check hardening disabled, all fields should be discoverable.
+ if (Reflection.canDiscoverFieldWithMetaReflection(klass, name, false) != true) {
+ throwDiscoveryException(klass, name, false,
+ "Meta reflection with hidden api hardening enabled", canDiscover);
+ }
+ }
+
if (canDiscover) {
// Test that modifiers are unaffected.
@@ -305,7 +331,8 @@
}
private static void checkMethod(Class<?> klass, String name, boolean isStatic,
- Visibility visibility, Behaviour behaviour, boolean invokesMemberCallback) throws Exception {
+ Visibility visibility, Behaviour behaviour, boolean invokesMemberCallback,
+ boolean testHiddenApiCheckHardeningDisabled) throws Exception {
boolean isPublic = (visibility == Visibility.Public);
if (klass.isInterface() && !isPublic) {
@@ -353,6 +380,23 @@
canDiscover);
}
+ // Check for meta reflection.
+
+ // With hidden api check hardening enabled, only white and light greylisted methods should be
+ // discoverable.
+ if (Reflection.canDiscoverMethodWithMetaReflection(klass, name, true) != canDiscover) {
+ throwDiscoveryException(klass, name, false,
+ "Meta reflection with hidden api hardening enabled", canDiscover);
+ }
+
+ if (testHiddenApiCheckHardeningDisabled) {
+ // With hidden api check hardening disabled, all methods should be discoverable.
+ if (Reflection.canDiscoverMethodWithMetaReflection(klass, name, false) != true) {
+ throwDiscoveryException(klass, name, false,
+ "Meta reflection with hidden api hardening enabled", canDiscover);
+ }
+ }
+
// Finish here if we could not discover the method.
if (canDiscover) {
@@ -381,7 +425,7 @@
}
private static void checkConstructor(Class<?> klass, Visibility visibility, Hiddenness hiddenness,
- Behaviour behaviour) throws Exception {
+ Behaviour behaviour, boolean testHiddenApiCheckHardeningDisabled) throws Exception {
boolean isPublic = (visibility == Visibility.Public);
String signature = "(" + visibility.mAssociatedType.mShorty +
@@ -436,6 +480,23 @@
canDiscover);
}
+ // Check for meta reflection.
+
+ // With hidden api check hardening enabled, only white and light greylisted constructors should
+ // be discoverable.
+ if (Reflection.canDiscoverConstructorWithMetaReflection(klass, args, true) != canDiscover) {
+ throwDiscoveryException(klass, fullName, false,
+ "Meta reflection with hidden api hardening enabled", canDiscover);
+ }
+
+ if (testHiddenApiCheckHardeningDisabled) {
+ // With hidden api check hardening disabled, all constructors should be discoverable.
+ if (Reflection.canDiscoverConstructorWithMetaReflection(klass, args, false) != true) {
+ throwDiscoveryException(klass, fullName, false,
+ "Meta reflection with hidden api hardening enabled", canDiscover);
+ }
+ }
+
if (canDiscover) {
// Test whether we can invoke the constructor.
diff --git a/test/674-hiddenapi/src-ex/Reflection.java b/test/674-hiddenapi/src-ex/Reflection.java
index 3667e91..173b9af 100644
--- a/test/674-hiddenapi/src-ex/Reflection.java
+++ b/test/674-hiddenapi/src-ex/Reflection.java
@@ -16,6 +16,7 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Arrays;
@@ -186,7 +187,45 @@
}
}
+ public static boolean canDiscoverMethodWithMetaReflection(Class<?> klass, String name,
+ boolean hardeningEnabled) {
+ try {
+ setHiddenApiCheckHardening(hardeningEnabled);
+ Method metaGetDeclaredMethod =
+ Class.class.getDeclaredMethod("getDeclaredMethod", String.class, Class[].class);
+ // Assumes method without parameters.
+ return ((Method)metaGetDeclaredMethod.invoke(klass, name, null)) != null;
+ } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ex) {
+ return false;
+ }
+ }
+
+ public static boolean canDiscoverFieldWithMetaReflection(Class<?> klass, String name,
+ boolean hardeningEnabled) {
+ try {
+ setHiddenApiCheckHardening(hardeningEnabled);
+ Method metaGetDeclaredField =
+ Class.class.getDeclaredMethod("getDeclaredField", String.class);
+ return ((Field)metaGetDeclaredField.invoke(klass, name)) != null;
+ } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ex) {
+ return false;
+ }
+ }
+
+ public static boolean canDiscoverConstructorWithMetaReflection(Class<?> klass, Class<?> args[],
+ boolean hardeningEnabled) {
+ try {
+ setHiddenApiCheckHardening(hardeningEnabled);
+ Method metaGetDeclaredConstructor =
+ Class.class.getDeclaredMethod("getDeclaredConstructor", Class[].class);
+ return ((Constructor<?>)metaGetDeclaredConstructor.invoke(klass, (Object)args)) != null;
+ } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ex) {
+ return false;
+ }
+ }
+
private static native int getHiddenApiAccessFlags();
+ private static native void setHiddenApiCheckHardening(boolean value);
public static boolean canObserveFieldHiddenAccessFlags(Class<?> klass, String name)
throws Exception {