From de6699692526f489db428b25f5be672b26cf7670 Mon Sep 17 00:00:00 2001 From: dfuchs Date: Mon, 1 Dec 2014 21:02:21 +0100 Subject: [PATCH] 8065552: setAccessible(true) on fields of Class may throw a SecurityException Summary: This fix hides the new private Class.classLoader field from reflection, rather than making it not accessible. Reviewed-by: mchung, coffeys --- src/share/classes/java/lang/Class.java | 2 + .../java/lang/reflect/AccessibleObject.java | 7 - src/share/classes/sun/reflect/Reflection.java | 1 + .../ClassDeclaredFieldsTest.java | 205 ++++++++++++++++++ 4 files changed, 208 insertions(+), 7 deletions(-) create mode 100644 test/java/lang/Class/getDeclaredField/ClassDeclaredFieldsTest.java diff --git a/src/share/classes/java/lang/Class.java b/src/share/classes/java/lang/Class.java index 43c8c0e8e..4c35cbce1 100644 --- a/src/share/classes/java/lang/Class.java +++ b/src/share/classes/java/lang/Class.java @@ -689,6 +689,8 @@ public final class Class implements java.io.Serializable, ClassLoader getClassLoader0() { return classLoader; } // Initialized in JVM not by private constructor + // This field is filtered from reflection access, i.e. getDeclaredField + // will throw NoSuchFieldException private final ClassLoader classLoader; /** diff --git a/src/share/classes/java/lang/reflect/AccessibleObject.java b/src/share/classes/java/lang/reflect/AccessibleObject.java index 0bf5e0eea..45e051c45 100644 --- a/src/share/classes/java/lang/reflect/AccessibleObject.java +++ b/src/share/classes/java/lang/reflect/AccessibleObject.java @@ -140,13 +140,6 @@ public class AccessibleObject implements AnnotatedElement { throw new SecurityException("Cannot make a java.lang.Class" + " constructor accessible"); } - } else if (obj instanceof Field && flag == true) { - Field f = (Field)obj; - if (f.getDeclaringClass() == Class.class && - f.getName().equals("classLoader")) { - throw new SecurityException("Cannot make java.lang.Class.classLoader" + - " accessible"); - } } obj.override = flag; } diff --git a/src/share/classes/sun/reflect/Reflection.java b/src/share/classes/sun/reflect/Reflection.java index 63baa42ab..bc3e13e23 100644 --- a/src/share/classes/sun/reflect/Reflection.java +++ b/src/share/classes/sun/reflect/Reflection.java @@ -46,6 +46,7 @@ public class Reflection { map.put(Reflection.class, new String[] {"fieldFilterMap", "methodFilterMap"}); map.put(System.class, new String[] {"security"}); + map.put(Class.class, new String[] {"classLoader"}); fieldFilterMap = map; methodFilterMap = new HashMap<>(); diff --git a/test/java/lang/Class/getDeclaredField/ClassDeclaredFieldsTest.java b/test/java/lang/Class/getDeclaredField/ClassDeclaredFieldsTest.java new file mode 100644 index 000000000..c05e1790d --- /dev/null +++ b/test/java/lang/Class/getDeclaredField/ClassDeclaredFieldsTest.java @@ -0,0 +1,205 @@ +/* + * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.lang.reflect.Field; +import java.lang.reflect.ReflectPermission; +import java.security.CodeSource; +import java.security.Permission; +import java.security.PermissionCollection; +import java.security.Permissions; +import java.security.Policy; +import java.security.ProtectionDomain; +import java.util.Arrays; +import java.util.Enumeration; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * @test + * @bug 8065552 + * @summary test that all fields returned by getDeclaredFields() can be + * set accessible if the right permission is granted; this test + * also verifies that Class.classLoader final private field is + * hidden from reflection access. + * @run main/othervm ClassDeclaredFieldsTest UNSECURE + * @run main/othervm ClassDeclaredFieldsTest SECURE + * + * @author danielfuchs + */ +public class ClassDeclaredFieldsTest { + + // Test with or without a security manager + public static enum TestCase { + UNSECURE, SECURE; + public void run() throws Exception { + System.out.println("Running test case: " + name()); + Configure.setUp(this); + test(this); + } + } + /** + * @param args the command line arguments + */ + public static void main(String[] args) throws Exception { + System.out.println(System.getProperty("java.version")); + if (args == null || args.length == 0) { + args = new String[] { "SECURE" }; + } else if (args.length != 1) { + throw new IllegalArgumentException("Only one arg expected: " + + Arrays.asList(args)); + } + TestCase.valueOf(args[0]).run(); + } + + static void test(TestCase test) { + for (Field f : Class.class.getDeclaredFields()) { + f.setAccessible(true); + System.out.println("Field "+f.getName()+" is now accessible."); + if (f.getName().equals("classLoader")) { + throw new RuntimeException("Found "+f.getName()+" field!"); + } + } + try { + Class.class.getDeclaredField("classLoader"); + throw new RuntimeException("Expected NoSuchFieldException for" + + " 'classLoader' field not raised"); + } catch(NoSuchFieldException x) { + System.out.println("Got expected exception: " + x); + } + System.out.println("Passed "+test); + } + + // A helper class to configure the security manager for the test, + // and bypass it when needed. + static class Configure { + static Policy policy = null; + static final ThreadLocal allowAll = new ThreadLocal() { + @Override + protected AtomicBoolean initialValue() { + return new AtomicBoolean(false); + } + }; + static void setUp(TestCase test) { + switch (test) { + case SECURE: + if (policy == null && System.getSecurityManager() != null) { + throw new IllegalStateException("SecurityManager already set"); + } else if (policy == null) { + policy = new SimplePolicy(TestCase.SECURE, allowAll); + Policy.setPolicy(policy); + System.setSecurityManager(new SecurityManager()); + } + if (System.getSecurityManager() == null) { + throw new IllegalStateException("No SecurityManager."); + } + if (policy == null) { + throw new IllegalStateException("policy not configured"); + } + break; + case UNSECURE: + if (System.getSecurityManager() != null) { + throw new IllegalStateException("SecurityManager already set"); + } + break; + default: + throw new InternalError("No such testcase: " + test); + } + } + static void doPrivileged(Runnable run) { + allowAll.get().set(true); + try { + run.run(); + } finally { + allowAll.get().set(false); + } + } + } + + // A Helper class to build a set of permissions. + final static class PermissionsBuilder { + final Permissions perms; + public PermissionsBuilder() { + this(new Permissions()); + } + public PermissionsBuilder(Permissions perms) { + this.perms = perms; + } + public PermissionsBuilder add(Permission p) { + perms.add(p); + return this; + } + public PermissionsBuilder addAll(PermissionCollection col) { + if (col != null) { + for (Enumeration e = col.elements(); e.hasMoreElements(); ) { + perms.add(e.nextElement()); + } + } + return this; + } + public Permissions toPermissions() { + final PermissionsBuilder builder = new PermissionsBuilder(); + builder.addAll(perms); + return builder.perms; + } + } + + // Policy for the test... + public static class SimplePolicy extends Policy { + + final Permissions permissions; + final Permissions allPermissions; + final ThreadLocal allowAll; // actually: this should be in a thread locale + public SimplePolicy(TestCase test, ThreadLocal allowAll) { + this.allowAll = allowAll; + // we don't actually need any permission to create our + // FileHandlers because we're passing invalid parameters + // which will make the creation fail... + permissions = new Permissions(); + permissions.add(new RuntimePermission("accessDeclaredMembers")); + permissions.add(new ReflectPermission("suppressAccessChecks")); + + // these are used for configuring the test itself... + allPermissions = new Permissions(); + allPermissions.add(new java.security.AllPermission()); + + } + + @Override + public boolean implies(ProtectionDomain domain, Permission permission) { + if (allowAll.get().get()) return allPermissions.implies(permission); + return permissions.implies(permission); + } + + @Override + public PermissionCollection getPermissions(CodeSource codesource) { + return new PermissionsBuilder().addAll(allowAll.get().get() + ? allPermissions : permissions).toPermissions(); + } + + @Override + public PermissionCollection getPermissions(ProtectionDomain domain) { + return new PermissionsBuilder().addAll(allowAll.get().get() + ? allPermissions : permissions).toPermissions(); + } + } + +} -- GitLab