You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by jw...@apache.org on 2017/06/04 23:17:41 UTC
[2/2] groovy git commit: GROOVY-8163: Prevent CachedField and
CachedMethod from leaking access permissions (closes #532)
GROOVY-8163: Prevent CachedField and CachedMethod from leaking access permissions (closes #532)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/b039c352
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/b039c352
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/b039c352
Branch: refs/heads/GROOVY_2_6_X
Commit: b039c352f2c815f262242a3a626c721f08f460a1
Parents: 14b3d46
Author: Dimitry Polivaev <dp...@gmx.de>
Authored: Sat Jun 3 20:03:18 2017 +0200
Committer: John Wagenleitner <jw...@apache.org>
Committed: Sun Jun 4 16:16:32 2017 -0700
----------------------------------------------------------------------
src/main/groovy/lang/MetaClassImpl.java | 3 +
.../reflection/AccessPermissionChecker.java | 83 ++++++
.../reflection/CacheAccessControlException.java | 27 ++
.../codehaus/groovy/reflection/CachedField.java | 2 +
.../groovy/reflection/CachedMethod.java | 8 +
.../groovy/reflection/SecurityTest.java | 271 +++++++++++++++++++
6 files changed, 394 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/b039c352/src/main/groovy/lang/MetaClassImpl.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/lang/MetaClassImpl.java b/src/main/groovy/lang/MetaClassImpl.java
index 2b15f76..736eae1 100644
--- a/src/main/groovy/lang/MetaClassImpl.java
+++ b/src/main/groovy/lang/MetaClassImpl.java
@@ -1832,6 +1832,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
} catch (IllegalArgumentException e) {
// can't access the field directly but there may be a getter
mp = null;
+ } catch (CacheAccessControlException e) {
+ // can't access the field directly but there may be a getter
+ mp = null;
}
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/b039c352/src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java b/src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java
new file mode 100644
index 0000000..73f35a9
--- /dev/null
+++ b/src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+package org.codehaus.groovy.reflection;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.ReflectPermission;
+import java.security.AccessControlException;
+
+import groovy.lang.GroovyObject;
+
+class AccessPermissionChecker {
+
+ private static final ReflectPermission REFLECT_PERMISSION = new ReflectPermission("suppressAccessChecks");
+
+ private static void checkAccessPermission(Class<?> declaringClass, final int modifiers, boolean isAccessible) {
+ final SecurityManager securityManager = System.getSecurityManager();
+ if (securityManager != null && isAccessible) {
+ if (((modifiers & Modifier.PRIVATE) != 0
+ || ((modifiers & (Modifier.PUBLIC | Modifier.PROTECTED)) == 0
+ && packageCanNotBeAddedAnotherClass(declaringClass)))
+ && !GroovyObject.class.isAssignableFrom(declaringClass)) {
+ securityManager.checkPermission(REFLECT_PERMISSION);
+ }
+ else if ((modifiers & (Modifier.PROTECTED)) != 0
+ && declaringClass.equals(ClassLoader.class)){
+ securityManager.checkCreateClassLoader();
+ }
+ }
+ }
+
+ private static boolean packageCanNotBeAddedAnotherClass(Class<?> declaringClass) {
+ return declaringClass.getName().startsWith("java.");
+ }
+
+ static void checkAccessPermission(Method method) {
+ try {
+ checkAccessPermission(method.getDeclaringClass(), method.getModifiers(), method.isAccessible());
+ } catch (AccessControlException e) {
+ throw createCacheAccessControlExceptionOf(method, e);
+ }
+ }
+
+ private static CacheAccessControlException createCacheAccessControlExceptionOf(Method method, AccessControlException e) {
+ return new CacheAccessControlException(
+ "Groovy object can not access method " + method.getName() + " cacheAccessControlExceptionOf class " + method.getDeclaringClass().getName()
+ + " with modifiers \"" + Modifier.toString(method.getModifiers()) + "\"", e);
+ }
+
+ static void checkAccessPermission(Field field) {
+ try {
+ checkAccessPermission(field.getDeclaringClass(), field.getModifiers(), field.isAccessible());
+ } catch (AccessControlException e) {
+ throw createCacheAccessControlExceptionOf(field, e);
+ }
+ }
+
+ private static CacheAccessControlException createCacheAccessControlExceptionOf(Field field, AccessControlException e) {
+ return new CacheAccessControlException(
+ "Groovy object can not access field " + field.getName() + " cacheAccessControlExceptionOf class " + field.getDeclaringClass().getName()
+ + " with modifiers \"" + Modifier.toString(field.getModifiers()) + "\"", e);
+ }
+
+ private AccessPermissionChecker() {}
+
+}
http://git-wip-us.apache.org/repos/asf/groovy/blob/b039c352/src/main/org/codehaus/groovy/reflection/CacheAccessControlException.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/reflection/CacheAccessControlException.java b/src/main/org/codehaus/groovy/reflection/CacheAccessControlException.java
new file mode 100644
index 0000000..b1438ec
--- /dev/null
+++ b/src/main/org/codehaus/groovy/reflection/CacheAccessControlException.java
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+package org.codehaus.groovy.reflection;
+
+import groovy.lang.GroovyRuntimeException;
+
+public class CacheAccessControlException extends GroovyRuntimeException{
+ public CacheAccessControlException(String message, Throwable cause) {
+ super(message, cause);
+ }
+}
http://git-wip-us.apache.org/repos/asf/groovy/blob/b039c352/src/main/org/codehaus/groovy/reflection/CachedField.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/reflection/CachedField.java b/src/main/org/codehaus/groovy/reflection/CachedField.java
index 543a373..a5ce0c2 100644
--- a/src/main/org/codehaus/groovy/reflection/CachedField.java
+++ b/src/main/org/codehaus/groovy/reflection/CachedField.java
@@ -50,6 +50,7 @@ public class CachedField extends MetaProperty {
* @throws RuntimeException if the property could not be evaluated
*/
public Object getProperty(final Object object) {
+ AccessPermissionChecker.checkAccessPermission(field);
try {
return field.get(object);
} catch (IllegalAccessException e) {
@@ -65,6 +66,7 @@ public class CachedField extends MetaProperty {
* @throws RuntimeException if the property could not be set
*/
public void setProperty(final Object object, Object newValue) {
+ AccessPermissionChecker.checkAccessPermission(field);
final Object goalValue = DefaultTypeTransformation.castToType(newValue, field.getType());
if (isFinal()) {
http://git-wip-us.apache.org/repos/asf/groovy/blob/b039c352/src/main/org/codehaus/groovy/reflection/CachedMethod.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/reflection/CachedMethod.java b/src/main/org/codehaus/groovy/reflection/CachedMethod.java
index 8c1c4a0..f49bad8 100644
--- a/src/main/org/codehaus/groovy/reflection/CachedMethod.java
+++ b/src/main/org/codehaus/groovy/reflection/CachedMethod.java
@@ -90,6 +90,12 @@ public class CachedMethod extends MetaMethod implements Comparable {
public final Object invoke(Object object, Object[] arguments) {
try {
+ AccessPermissionChecker.checkAccessPermission(cachedMethod);
+ }
+ catch (CacheAccessControlException ex) {
+ throw new InvokerInvocationException(ex);
+ }
+ try {
return cachedMethod.invoke(object, arguments);
} catch (IllegalArgumentException e) {
throw new InvokerInvocationException(e);
@@ -124,6 +130,7 @@ public class CachedMethod extends MetaMethod implements Comparable {
}
public final Method setAccessible() {
+ AccessPermissionChecker.checkAccessPermission(cachedMethod);
// if (queuedToCompile.compareAndSet(false,true)) {
// if (isCompilable())
// CompileThread.addMethod(this);
@@ -324,6 +331,7 @@ public class CachedMethod extends MetaMethod implements Comparable {
}
public Method getCachedMethod() {
+ AccessPermissionChecker.checkAccessPermission(cachedMethod);
return cachedMethod;
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/b039c352/src/test/org/codehaus/groovy/reflection/SecurityTest.java
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/reflection/SecurityTest.java b/src/test/org/codehaus/groovy/reflection/SecurityTest.java
new file mode 100644
index 0000000..2f76f03
--- /dev/null
+++ b/src/test/org/codehaus/groovy/reflection/SecurityTest.java
@@ -0,0 +1,271 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+package org.codehaus.groovy.reflection;
+
+import groovy.lang.GroovyObjectSupport;
+import groovy.util.GroovyTestCase;
+import org.codehaus.groovy.runtime.InvokerInvocationException;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.lang.reflect.ReflectPermission;
+import java.nio.ByteBuffer;
+import java.security.AccessControlException;
+import java.security.Permission;
+import java.security.Permissions;
+import java.security.ProtectionDomain;
+
+public class SecurityTest extends GroovyTestCase {
+
+ @SuppressWarnings("unused")
+ public class TestClass{
+ public String publicField;
+ protected String protectedField;
+ String packagePrivateField;
+ private String privateField;
+
+ private boolean methodCalled = false;
+
+ public void publicMethod() {
+ privateMethod();
+ }
+
+ private void privateMethod() {
+ methodCalled = true;
+ }
+
+ void packagePrivateMethod() {
+ privateMethod();
+ }
+
+ void protectedMethod() {
+ privateMethod();
+ }
+
+ public boolean isMethodCalled() {
+ return methodCalled;
+ }
+ }
+
+ @SuppressWarnings("unused")
+ public class TestGroovyClass extends GroovyObjectSupport{
+ private String privateField;
+ private boolean methodCalled = false;
+ private void privateMethod() {
+ methodCalled = true;
+ }
+ public boolean isMethodCalled() {
+ return methodCalled;
+ }
+ }
+ SecurityManager restrictiveSecurityManager;
+ CachedMethod cachedMethodUnderTest;
+ CachedField cachedFieldUnderTest;
+ Permissions forbidden;
+
+ public void setUp() {
+ forbidden = new Permissions();
+ forbidden.add(new ReflectPermission("suppressAccessChecks"));
+ restrictiveSecurityManager = new SecurityManager() {
+
+ @Override
+ public void checkPermission(Permission perm) {
+ if (forbidden.implies(perm))
+ throw new AccessControlException(perm.getName());
+ }
+ };
+ }
+
+ public void tearDown(){
+ System.setSecurityManager(null);
+ }
+
+ private CachedMethod createCachedMethod(String name) throws Exception {
+ return createCachedMethod(TestClass.class, name);
+ }
+
+ private CachedMethod createCachedMethod(Class<?> cachedClass, String methodName, Class... parameters) throws NoSuchMethodException {
+ Method method = cachedClass.getDeclaredMethod(methodName, parameters);
+ method.setAccessible(true);
+ return new CachedMethod(null, method);
+ }
+
+ private boolean invokesCachedMethod() {
+ TestClass object = new TestClass();
+ cachedMethodUnderTest.invoke(object, new Object[]{});
+ return object.isMethodCalled();
+ }
+
+ private CachedField createCachedField(String name) throws Exception {
+ Field field = TestClass.class.getDeclaredField(name);
+ field.setAccessible(true);
+ return new CachedField(field);
+ }
+
+ public void testInvokesPublicMethodsWithoutChecks() throws Exception {
+ cachedMethodUnderTest = createCachedMethod("publicMethod");
+ System.setSecurityManager(restrictiveSecurityManager);
+ assertTrue(invokesCachedMethod());
+ }
+
+ public void testReturnsAccesiblePublicMethodsWithoutChecks() throws Exception {
+ cachedMethodUnderTest = createCachedMethod("publicMethod");
+ System.setSecurityManager(restrictiveSecurityManager);
+ assertEquals("publicMethod", cachedMethodUnderTest.setAccessible().getName());
+ assertEquals("publicMethod", cachedMethodUnderTest.getCachedMethod().getName());
+ }
+
+ public void testAccessesPublicFieldsWithoutChecks() throws Exception {
+ cachedFieldUnderTest = createCachedField("publicField");
+ System.setSecurityManager(restrictiveSecurityManager);
+ TestClass object = new TestClass();
+ cachedFieldUnderTest.setProperty(object, "value");
+ assertEquals("value", cachedFieldUnderTest.getProperty(object));
+ }
+
+ public void testInvokesPrivateMethodsWithoutSecurityManager() throws Exception{
+ cachedMethodUnderTest = createCachedMethod("privateMethod");
+ assertTrue(invokesCachedMethod());
+ }
+
+ public void testAccessesPrivateFieldsWithoutSecurityManager() throws Exception {
+ cachedFieldUnderTest = createCachedField("privateField");
+ System.setSecurityManager(null);
+ TestClass object = new TestClass();
+ cachedFieldUnderTest.setProperty(object, "value");
+ assertEquals("value", cachedFieldUnderTest.getProperty(object));
+ }
+
+ public void testReturnsAccesiblePrivateMethodsWithoutSecurityManager() throws Exception {
+ cachedMethodUnderTest = createCachedMethod("privateMethod");
+ System.setSecurityManager(null);
+ assertEquals("privateMethod", cachedMethodUnderTest.setAccessible().getName());
+ assertEquals("privateMethod", cachedMethodUnderTest.getCachedMethod().getName());
+ }
+
+ public void testChecksReflectPermissionForInvokeOnPrivateMethods() throws Exception {
+ cachedMethodUnderTest = createCachedMethod("privateMethod");
+ System.setSecurityManager(restrictiveSecurityManager);
+ try {
+ invokesCachedMethod();
+ fail();
+ }
+ catch (InvokerInvocationException e) {
+ assertEquals(CacheAccessControlException.class, e.getCause().getClass());
+ }
+ }
+
+ public void testChecksReflectPermissionForFieldAccessOnPrivateFields() throws Exception {
+ cachedFieldUnderTest = createCachedField("privateField");
+ System.setSecurityManager(restrictiveSecurityManager);
+ TestClass object = new TestClass();
+ try {
+ cachedFieldUnderTest.setProperty(object, "value");
+ fail();
+ }
+ catch (CacheAccessControlException e) {
+ }
+
+ try {
+ cachedFieldUnderTest.getProperty(object);
+ fail();
+ }
+ catch (CacheAccessControlException e) {
+ }
+ }
+
+ public void testChecksReflectPermissionForMethodAccessOnPrivateMethods() throws Exception {
+ cachedMethodUnderTest = createCachedMethod("privateMethod");
+ System.setSecurityManager(restrictiveSecurityManager);
+ try {
+ cachedMethodUnderTest.setAccessible();
+ fail();
+ }
+ catch (CacheAccessControlException e) {
+ }
+
+ try {
+ cachedMethodUnderTest.getCachedMethod();
+ fail();
+ }
+ catch (CacheAccessControlException e) {
+ }
+ }
+
+ public void testInvokesPackagePrivateMethodsWithoutChecksInNonRestrictedPackages() throws Exception {
+ cachedMethodUnderTest = createCachedMethod("packagePrivateMethod");
+ System.setSecurityManager(restrictiveSecurityManager);
+ assertTrue(invokesCachedMethod());
+ }
+
+ public void testChecksReflectPermissionForInvokeOnPackagePrivateMethodsInRestrictedJavaPackages() throws Exception {
+ cachedMethodUnderTest = createCachedMethod(ClassLoader.class, "getBootstrapClassPath", new Class[0]);
+ System.setSecurityManager(restrictiveSecurityManager);
+
+ try {
+ cachedMethodUnderTest.invoke(null, new Object[]{});
+ fail();
+ }
+ catch (InvokerInvocationException e) {
+ assertEquals(CacheAccessControlException.class, e.getCause().getClass());
+ }
+ }
+
+ public void testInvokesProtectedMethodsWithoutChecks() throws Exception {
+ cachedMethodUnderTest = createCachedMethod("protectedMethod");
+ System.setSecurityManager(restrictiveSecurityManager);
+ assertTrue(invokesCachedMethod());
+ }
+
+
+ public void testChecksCreateClassLoaderPermissionForClassLoaderProtectedMethodAccess() throws Exception {
+ cachedMethodUnderTest = createCachedMethod(ClassLoader.class, "defineClass", new Class[]{String.class, ByteBuffer.class, ProtectionDomain.class});
+ forbidden = new Permissions();
+ forbidden.add(new RuntimePermission("createClassLoader"));
+ System.setSecurityManager(restrictiveSecurityManager);
+
+ ClassLoader classLoader = getClass().getClassLoader();
+
+ try {
+ cachedMethodUnderTest.invoke(classLoader, new Object[]{null, null, null});
+ fail();
+ }
+ catch (InvokerInvocationException e) {
+ assertEquals(CacheAccessControlException.class, e.getCause().getClass());
+ }
+ }
+
+ public void testInvokesPrivateMethodsInGroovyObjectsWithoutChecks() throws Exception {
+ cachedMethodUnderTest = createCachedMethod(TestGroovyClass.class, "privateMethod");
+ TestGroovyClass object = new TestGroovyClass();
+ System.setSecurityManager(restrictiveSecurityManager);
+ cachedMethodUnderTest.invoke(object, new Object[]{});
+ assertTrue(object.isMethodCalled());
+ }
+
+ public void testAccessesPrivateFieldsInGroovyObjectsWithoutChecks() throws Exception {
+ Field field = TestGroovyClass.class.getDeclaredField("privateField");
+ field.setAccessible(true);
+ cachedFieldUnderTest = new CachedField(field);
+ TestGroovyClass object = new TestGroovyClass();
+ System.setSecurityManager(restrictiveSecurityManager);
+ cachedFieldUnderTest.setProperty(object, "value");
+ assertEquals("value", cachedFieldUnderTest.getProperty(object));
+ }
+
+}