groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jwagenleit...@apache.org
Subject [2/2] groovy git commit: GROOVY-8163: Prevent CachedField and CachedMethod from leaking access permissions (closes #532)
Date Sun, 04 Jun 2017 23:18:40 GMT
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/0305a38a
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/0305a38a
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/0305a38a

Branch: refs/heads/GROOVY_2_5_X
Commit: 0305a38a0cc8f4190a1486c460ebc6f712ad1a07
Parents: d00a10f
Author: Dimitry Polivaev <dpolivaev@gmx.de>
Authored: Sat Jun 3 20:03:18 2017 +0200
Committer: John Wagenleitner <jwagenleitner@apache.org>
Committed: Sun Jun 4 16:18:02 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/0305a38a/src/main/groovy/lang/MetaClassImpl.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/lang/MetaClassImpl.java b/src/main/groovy/lang/MetaClassImpl.java
index a059a4f..c6612ac 100644
--- a/src/main/groovy/lang/MetaClassImpl.java
+++ b/src/main/groovy/lang/MetaClassImpl.java
@@ -1814,6 +1814,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/0305a38a/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/0305a38a/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/0305a38a/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/0305a38a/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/0305a38a/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));
+    }
+
+}


Mime
View raw message