From 78010e94a115e1a5c3e58fb68c2b0d361d5de675 Mon Sep 17 00:00:00 2001 From: Dimitry Polivaev Date: Sat, 3 Jun 2017 20:03:18 +0200 Subject: [PATCH] Prevent CachedField and CachedMethod from leaking access permissions --- src/main/groovy/lang/MetaClassImpl.java | 3 + .../reflection/AccessPermissionChecker.java | 83 ++++++ .../CacheAccessControlException.java | 27 ++ .../groovy/reflection/CachedField.java | 2 + .../groovy/reflection/CachedMethod.java | 8 + .../groovy/reflection/SecurityTest.java | 271 ++++++++++++++++++ 6 files changed, 394 insertions(+) create mode 100644 src/main/org/codehaus/groovy/reflection/AccessPermissionChecker.java create mode 100644 src/main/org/codehaus/groovy/reflection/CacheAccessControlException.java create mode 100644 src/test/org/codehaus/groovy/reflection/SecurityTest.java diff --git a/src/main/groovy/lang/MetaClassImpl.java b/src/main/groovy/lang/MetaClassImpl.java index 2b15f76e25b..736eae15b45 100644 --- a/src/main/groovy/lang/MetaClassImpl.java +++ b/src/main/groovy/lang/MetaClassImpl.java @@ -1832,6 +1832,9 @@ public Object getProperty(Class sender, Object object, String name, boolean useS } 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; } } 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 00000000000..73f35a912e0 --- /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() {} + +} 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 00000000000..b1438eca7a4 --- /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); + } +} diff --git a/src/main/org/codehaus/groovy/reflection/CachedField.java b/src/main/org/codehaus/groovy/reflection/CachedField.java index 543a3736533..a5ce0c243f4 100644 --- a/src/main/org/codehaus/groovy/reflection/CachedField.java +++ b/src/main/org/codehaus/groovy/reflection/CachedField.java @@ -50,6 +50,7 @@ public int getModifiers() { * @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 Object getProperty(final Object object) { * @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()) { diff --git a/src/main/org/codehaus/groovy/reflection/CachedMethod.java b/src/main/org/codehaus/groovy/reflection/CachedMethod.java index 8c1c4a03a52..f49bad8cf1e 100644 --- a/src/main/org/codehaus/groovy/reflection/CachedMethod.java +++ b/src/main/org/codehaus/groovy/reflection/CachedMethod.java @@ -89,6 +89,12 @@ public CachedClass getDeclaringClass() { } 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) { @@ -124,6 +130,7 @@ public String getSignature() { } public final Method setAccessible() { + AccessPermissionChecker.checkAccessPermission(cachedMethod); // if (queuedToCompile.compareAndSet(false,true)) { // if (isCompilable()) // CompileThread.addMethod(this); @@ -324,6 +331,7 @@ else if (o2 instanceof CachedMethod) } public Method getCachedMethod() { + AccessPermissionChecker.checkAccessPermission(cachedMethod); return cachedMethod; } 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 00000000000..2f76f032e90 --- /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)); + } + +}