groovy-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitry Polivaev (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (GROOVY-8163) Groovy scripts can disable java security manager and escape sandbox
Date Sun, 23 Apr 2017 10:53:04 GMT

     [ https://issues.apache.org/jira/browse/GROOVY-8163?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Dimitry Polivaev updated GROOVY-8163:
-------------------------------------
    Description: 
Consider following test

{code}
package groovytest;

import groovy.util.Eval;
import org.junit.*;

import java.net.URL;
import java.security.AccessController;
import java.security.PrivilegedAction;

public class GroovySecurityTest {

	public static final String RESTRICTED_PERMISSIONS_FOR_SCRIPT_ONLY_POLICY = "/restrictedPermissionsForScriptOnlyPolicy.txt";

	public static final String POLICY = RESTRICTED_PERMISSIONS_FOR_SCRIPT_ONLY_POLICY;

	@BeforeClass
	public static void setPolicy() throws Exception {
		final String dirTest = GroovySecurityTest.class.getProtectionDomain().getCodeSource().getLocation().toString();
		final String dirGroovy = Eval.class.getProtectionDomain().getCodeSource().getLocation().toString();
		System.setProperty("dir.test",dirTest + "-");
		System.setProperty("dir.groovy",dirGroovy);
		final URL policy = GroovySecurityTest.class.getResource(POLICY);
		System.setProperty("java.security.policy", policy.toString());
	}
	
	
	@Before
	public void setSecurityManager() throws Exception {
			System.setSecurityManager(new SecurityManager());
	}

	@After
	public void removeSecurityManager() throws Exception {
		AccessController.doPrivileged(new PrivilegedAction<Void>() {
			@Override
			public Void run() {
				System.setSecurityManager(null);
				return null;
			}
		});
	}

	@Test
	public void doesNotChangeScriptPermissionsUsungPrivateFieldAccess() throws Exception {
		try {
			AccessController.doPrivileged(new PrivilegedAction<Void>() {

                @Override
                public Void run() {
                    Eval.me("getClass().protectionDomain0.hasAllPerm = true;"
                                    + "System.setSecurityManager(null);"
                                    + "1");
                    return null;
                }
            });
		} catch (Exception e) {
		}

		Assert.assertNotNull(System.getSecurityManager());
	}

}
{code}

with following policy file restrictedPermissionsForScriptOnlyPolicy.txt

{code}
grant codeBase "${dir.test}" {
	permission java.security.AllPermission;
};

grant codeBase "${dir.groovy}" {
	permission java.security.AllPermission;
};

grant {
};

{code}

It fails: security manager is not set any more when the test assertion is checked.
It happens because CachedField from org.codehaus.groovy.reflection is created withing trusted
code base (groovy jar) and gives access to the field to untrusted scripts without any security
checks. The same problem relates to CachedMethod which would allow any script to access protected
method java.lang.ClassLoader#defineClass(java.lang.String, byte[], int, int, java.security.ProtectionDomain)
that can be misused to manipulate code sources of classes loaded from script to give them
all permissions.

It also appears that if I remove permissions from groovy.jar using more restrictive policy
using following policy file restrictedPermissionsPolicy.txt
{code}
grant  codeBase "${dir.test}" {
    permission java.lang.RuntimePermission "*";
    permission java.security.SecurityPermission "*";
     permission java.io.FilePermission "<<ALL FILES>>", "read";
    permission java.util.PropertyPermission "*", "read";
    permission groovy.security.GroovyCodeSourcePermission "*";
};

grant  codeBase "${dir.groovy}" {
    permission java.lang.RuntimePermission "*";
    permission java.security.SecurityPermission "*";
    permission java.io.FilePermission "<<ALL FILES>>", "read";
    permission java.util.PropertyPermission "*", "read";
    permission groovy.security.GroovyCodeSourcePermission "*";
};

grant {
    permission java.lang.RuntimePermission "accessDeclaredMembers";
};

{code}

it has a consequence that groovy can not access even some public methods on bean properties
as shown in the following test

{code}
package groovytest;

import groovy.util.Eval;
import org.junit.*;

import java.net.URL;
import java.security.AccessController;
import java.security.PrivilegedAction;

public class GroovyBeanTest {

	public static final String RESTRICTED_PERMISSIONS_POLICY = "/restrictedPermissionsPolicy.txt";

	public static final String POLICY = RESTRICTED_PERMISSIONS_POLICY;

	@BeforeClass
	public static void setPolicy() throws Exception {
		final String dirTest = GroovyBeanTest.class.getProtectionDomain().getCodeSource().getLocation().toString();
		final String dirGroovy = Eval.class.getProtectionDomain().getCodeSource().getLocation().toString();
		System.setProperty("dir.test",dirTest + "-");
		System.setProperty("dir.groovy",dirGroovy);
		final URL policy = GroovyBeanTest.class.getResource(POLICY);
		System.setProperty("java.security.policy", policy.toString());
	}
	
	
	@Before
	public void setSecurityManager() throws Exception {
			System.setSecurityManager(new SecurityManager());
	}

	@After
	public void removeSecurityManager() throws Exception {
		AccessController.doPrivileged(new PrivilegedAction<Void>() {
			@Override
			public Void run() {
				System.setSecurityManager(null);
				return null;
			}
		});
	}
	
	public interface BeanInterface{
		public String getName();
	}

	private class Bean implements BeanInterface{
		private String _name = "bean";
		public String getName(){
			return _name;
		}

	}

	@Test
	public void returnsBeanPropertyValue() throws Exception {
		AccessController.doPrivileged(new PrivilegedAction<Void>() {

			@Override
			public Void run() {
				Assert.assertEquals("bean", Eval.x(new Bean(), "x.name"));
				return null;
			}
		});
	}


	static class BeanClass {
		private String name = "bean";
	}

	@Test
	public void returnsBeanClassName() throws Exception {
		AccessController.doPrivileged(new PrivilegedAction<Void>() {

			@Override
			public Void run() {
				Assert.assertEquals(BeanClass.class.getName(), Eval.x(new BeanClass(), "x.class.name"));
				return null;
			}
		});
	}
}
{code}

The both tests fail as the bean properties can not be found by groovy.
It turned out that I can not run the both tests in one process, make sure you run them separately.

In order to fix this issue for my open source project Freeplane I have to patch groovy code.
It turned out to be possible. 
So I want to share the fix with you and ask you to integrate it.

The fix is based on groovy version 2.4.9 and I think that it can be applied to any Groovy
version.

You only need to check for access permissions at the relevant places to make sure that they
are not leaked from groovy (which needs them to work properly) to the untrusted script

{code}
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 groovy.lang.GroovyObject;

class AccessPermissionChecker {

	private static final ReflectPermission REFLECT_PERMISSION = new ReflectPermission("suppressAccessChecks");

	static private void checkAccessPermission(Class<?> declaringClass, final int modifiers,
boolean isAccessible) {
		final SecurityManager securityManager = System.getSecurityManager();
		if (isAccessible && securityManager != null) {
				if ((modifiers & (Modifier.PUBLIC | Modifier.PROTECTED)) == 0
						&& !GroovyObject.class.isAssignableFrom(declaringClass)) {
                        securityManager.checkPermission(REFLECT_PERMISSION);
                }
                else if ((modifiers & (Modifier.PUBLIC)) == 0
					&& declaringClass.equals(ClassLoader.class)){
					securityManager.checkCreateClassLoader();
				}
		}
	}

	static public void checkAccessPermission(Method method) {
		checkAccessPermission(method.getDeclaringClass(), method.getModifiers(), method.isAccessible()
		);
	}

	public static void checkAccessPermission(Field field) {
		checkAccessPermission(field.getDeclaringClass(), field.getModifiers(), field.isAccessible()
		);
	}
}
{code}

patching your classes as follows:

CachedField.java:

{code}
    /**
     * @return the property of the given object
     * @throws RuntimeException if the property could not be evaluated
     */
    public Object getProperty(final Object object) {
        try {
            AccessPermissionChecker.checkAccessPermission(field);
        }
        catch (AccessControlException ex) {
            throw new IllegalArgumentException("Illegal access to field" + " " + field.getName());
        }
        try {
            return field.get(object);
        } catch (IllegalAccessException e) {
            throw new GroovyRuntimeException("Cannot get the property '" + name + "'.", e);
        }
    }

    /**
     * Sets the property on the given object to the new value
     *
     * @param object on which to set the property
     * @param newValue the new value of the property
     * @throws RuntimeException if the property could not be set
     */
    public void setProperty(final Object object, Object newValue) {
        try {
            AccessPermissionChecker.checkAccessPermission(field);
        }
        catch (AccessControlException ex) {
            throw new IllegalArgumentException("Illegal access to field" + " " + field.getName());
        }
        final Object goalValue = DefaultTypeTransformation.castToType(newValue, field.getType());

        if (isFinal()) {
            throw new GroovyRuntimeException("Cannot set the property '" + name + "' because
the backing field is final.");
        }
        try {
            field.set(object, goalValue);
        } catch (IllegalAccessException ex) {
            throw new GroovyRuntimeException("Cannot set the property '" + name + "'.", ex);
        }
    }
{code}

CachedMethod.java:
{code}
    public final Object invoke(Object object, Object[] arguments) {
        try {
            AccessPermissionChecker.checkAccessPermission(cachedMethod);
        }
        catch (AccessControlException ex) {
            throw new InvokerInvocationException(new IllegalArgumentException("Illegal access
to method" + cachedMethod.getName()));
        }
        try {
            return cachedMethod.invoke(object, arguments);
        } catch (IllegalArgumentException e) {
            throw new InvokerInvocationException(e);
        } catch (IllegalAccessException e) {
            throw new InvokerInvocationException(e);
        } catch (InvocationTargetException e) {
            Throwable cause = e.getCause();
            throw (cause instanceof RuntimeException && !(cause instanceof MissingMethodException))
?
                    (RuntimeException) cause : new InvokerInvocationException(e);
        }
    }

    public final Method setAccessible() {
        try {
            AccessPermissionChecker.checkAccessPermission(cachedMethod);
        }
        catch (AccessControlException ex) {
            throw new IllegalArgumentException("Illegal access to method" + cachedMethod.getName());
        }
//        if (queuedToCompile.compareAndSet(false,true)) {
//            if (isCompilable())
//              CompileThread.addMethod(this);
//        }
        return cachedMethod;
    }

    public Method getCachedMethod() {
        try {
            AccessPermissionChecker.checkAccessPermission(cachedMethod);
        }
        catch (AccessControlException ex) {
            throw new IllegalArgumentException("Illegal access to method" + cachedMethod.getName());
        }
        return cachedMethod;
    }
}

{code}

In order to apply the patch in Freeplane I created a separate project https://github.com/dpolivaev/securegroovy
which generates the patch at runtime using bytebuddy. 

I would appreciate if you could integrate the patch in groovy code.

There is one subtle issue with AccessPermissionChecker: as you see it allows access to all
protected methods except for ClassLoader without any further checks.
But for the protected methods from ClassLoader is makes one additional check: Otherwise a
script could access a class loader by getClass().getClassLoader() and misuse its defineClass
method to let malicious code appear trusted. 

  was:
Consider following test

{code}
package groovytest;

import groovy.util.Eval;
import org.junit.*;

import java.net.URL;
import java.security.AccessController;
import java.security.PrivilegedAction;

public class GroovySecurityTest {

	public static final String RESTRICTED_PERMISSIONS_FOR_SCRIPT_ONLY_POLICY = "/restrictedPermissionsForScriptOnlyPolicy.txt";

	public static final String POLICY = RESTRICTED_PERMISSIONS_FOR_SCRIPT_ONLY_POLICY;

	@BeforeClass
	public static void setPolicy() throws Exception {
		final String dirTest = GroovySecurityTest.class.getProtectionDomain().getCodeSource().getLocation().toString();
		final String dirGroovy = Eval.class.getProtectionDomain().getCodeSource().getLocation().toString();
		System.setProperty("dir.test",dirTest + "-");
		System.setProperty("dir.groovy",dirGroovy);
		final URL policy = GroovySecurityTest.class.getResource(POLICY);
		System.setProperty("java.security.policy", policy.toString());
	}
	
	
	@Before
	public void setSecurityManager() throws Exception {
			System.setSecurityManager(new SecurityManager());
	}

	@After
	public void removeSecurityManager() throws Exception {
		AccessController.doPrivileged(new PrivilegedAction<Void>() {
			@Override
			public Void run() {
				System.setSecurityManager(null);
				return null;
			}
		});
	}

	@Test
	public void doesNotChangeScriptPermissionsUsungPrivateFieldAccess() throws Exception {
		try {
			AccessController.doPrivileged(new PrivilegedAction<Void>() {

                @Override
                public Void run() {
                    Eval.me("getClass().protectionDomain0.hasAllPerm = true;"
                                    + "System.setSecurityManager(null);"
                                    + "1");
                    return null;
                }
            });
		} catch (Exception e) {
		}

		Assert.assertNotNull(System.getSecurityManager());
	}

}
{code}

with following policy file restrictedPermissionsForScriptOnlyPolicy.txt

{code}
grant codeBase "${dir.test}" {
	permission java.security.AllPermission;
};

grant codeBase "${dir.groovy}" {
	permission java.security.AllPermission;
};

grant {
};

{code}

It fails: security manager is not set any more when the test assertion is checked.
It happens because CachedField from org.codehaus.groovy.reflection is created withing trusted
code base (groovy jar) and gives access to the field to untrusted scripts without any security
checks. The same problem relates to CachedMethod which would allow any script to access protected
method java.lang.ClassLoader#defineClass(java.lang.String, byte[], int, int, java.security.ProtectionDomain)
that can be misused to manipulate code sources of classes loaded from script to give them
all permissions.

It also appears that if I remove permissions from groovy.jar using more restrictive policy
using following policy file restrictedPermissionsPolicy.txt
{code}
grant  codeBase "${dir.test}" {
    permission java.lang.RuntimePermission "*";
    permission java.security.SecurityPermission "*";
     permission java.io.FilePermission "<<ALL FILES>>", "read";
    permission java.util.PropertyPermission "*", "read";
    permission groovy.security.GroovyCodeSourcePermission "*";
};

grant  codeBase "${dir.groovy}" {
    permission java.lang.RuntimePermission "*";
    permission java.security.SecurityPermission "*";
    permission java.io.FilePermission "<<ALL FILES>>", "read";
    permission java.util.PropertyPermission "*", "read";
    permission groovy.security.GroovyCodeSourcePermission "*";
};

grant {
    permission java.lang.RuntimePermission "accessDeclaredMembers";
};

{code}

it has a consequence that groovy can not access even some public methods on bean properties
as shown in the following test

{code}
package groovytest;

import groovy.util.Eval;
import org.junit.*;

import java.net.URL;
import java.security.AccessController;
import java.security.PrivilegedAction;

public class GroovyBeanTest {

	public static final String RESTRICTED_PERMISSIONS_POLICY = "/restrictedPermissionsPolicy.txt";

	public static final String POLICY = RESTRICTED_PERMISSIONS_POLICY;

	@BeforeClass
	public static void setPolicy() throws Exception {
		final String dirTest = GroovyBeanTest.class.getProtectionDomain().getCodeSource().getLocation().toString();
		final String dirGroovy = Eval.class.getProtectionDomain().getCodeSource().getLocation().toString();
		System.setProperty("dir.test",dirTest + "-");
		System.setProperty("dir.groovy",dirGroovy);
		final URL policy = GroovyBeanTest.class.getResource(POLICY);
		System.setProperty("java.security.policy", policy.toString());
	}
	
	
	@Before
	public void setSecurityManager() throws Exception {
			System.setSecurityManager(new SecurityManager());
	}

	@After
	public void removeSecurityManager() throws Exception {
		AccessController.doPrivileged(new PrivilegedAction<Void>() {
			@Override
			public Void run() {
				System.setSecurityManager(null);
				return null;
			}
		});
	}
	
	public interface BeanInterface{
		public String getName();
	}

	private class Bean implements BeanInterface{
		private String _name = "bean";
		public String getName(){
			return _name;
		}

	}

	@Test
	public void returnsBeanPropertyValue() throws Exception {
		AccessController.doPrivileged(new PrivilegedAction<Void>() {

			@Override
			public Void run() {
				Assert.assertEquals("bean", Eval.x(new Bean(), "x.name"));
				return null;
			}
		});
	}


	static class BeanClass {
		private String name = "bean";
	}

	@Test
	public void returnsBeanClassName() throws Exception {
		AccessController.doPrivileged(new PrivilegedAction<Void>() {

			@Override
			public Void run() {
				Assert.assertEquals(BeanClass.class.getName(), Eval.x(new BeanClass(), "x.class.name"));
				return null;
			}
		});
	}
}
{code}

The both tests fail as the bean properties can not be found by groovy.
It turned out that I can not run the both tests in one process, make sure you run them separately.

In order to fix this issue for my open source project Freeplane I have to patch groovy code.
It turned out to be possible. 
So I want to share the fix with you and ask you to integrate it.

The fix is based on groovy version 2.4.9 and I think that it can be applied to any Groovy
version.

You only need to check for access permissions at the relevant places to make sure that they
are not leaked to the script.

{code}
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 groovy.lang.GroovyObject;

class AccessPermissionChecker {

	private static final ReflectPermission REFLECT_PERMISSION = new ReflectPermission("suppressAccessChecks");

	static private void checkAccessPermission(Class<?> declaringClass, final int modifiers,
boolean isAccessible) {
		final SecurityManager securityManager = System.getSecurityManager();
		if (isAccessible && securityManager != null) {
				if ((modifiers & (Modifier.PUBLIC | Modifier.PROTECTED)) == 0
						&& !GroovyObject.class.isAssignableFrom(declaringClass)) {
                        securityManager.checkPermission(REFLECT_PERMISSION);
                }
                else if ((modifiers & (Modifier.PUBLIC)) == 0
					&& declaringClass.equals(ClassLoader.class)){
					securityManager.checkCreateClassLoader();
				}
		}
	}

	static public void checkAccessPermission(Method method) {
		checkAccessPermission(method.getDeclaringClass(), method.getModifiers(), method.isAccessible()
		);
	}

	public static void checkAccessPermission(Field field) {
		checkAccessPermission(field.getDeclaringClass(), field.getModifiers(), field.isAccessible()
		);
	}
}
{code}

patching your classes as follows:

CachedField.java:

{code}
    /**
     * @return the property of the given object
     * @throws RuntimeException if the property could not be evaluated
     */
    public Object getProperty(final Object object) {
        try {
            AccessPermissionChecker.checkAccessPermission(field);
        }
        catch (AccessControlException ex) {
            throw new IllegalArgumentException("Illegal access to field" + " " + field.getName());
        }
        try {
            return field.get(object);
        } catch (IllegalAccessException e) {
            throw new GroovyRuntimeException("Cannot get the property '" + name + "'.", e);
        }
    }

    /**
     * Sets the property on the given object to the new value
     *
     * @param object on which to set the property
     * @param newValue the new value of the property
     * @throws RuntimeException if the property could not be set
     */
    public void setProperty(final Object object, Object newValue) {
        try {
            AccessPermissionChecker.checkAccessPermission(field);
        }
        catch (AccessControlException ex) {
            throw new IllegalArgumentException("Illegal access to field" + " " + field.getName());
        }
        final Object goalValue = DefaultTypeTransformation.castToType(newValue, field.getType());

        if (isFinal()) {
            throw new GroovyRuntimeException("Cannot set the property '" + name + "' because
the backing field is final.");
        }
        try {
            field.set(object, goalValue);
        } catch (IllegalAccessException ex) {
            throw new GroovyRuntimeException("Cannot set the property '" + name + "'.", ex);
        }
    }
{code}

CachedMethod.java:
{code}
    public final Object invoke(Object object, Object[] arguments) {
        try {
            AccessPermissionChecker.checkAccessPermission(cachedMethod);
        }
        catch (AccessControlException ex) {
            throw new InvokerInvocationException(new IllegalArgumentException("Illegal access
to method" + cachedMethod.getName()));
        }
        try {
            return cachedMethod.invoke(object, arguments);
        } catch (IllegalArgumentException e) {
            throw new InvokerInvocationException(e);
        } catch (IllegalAccessException e) {
            throw new InvokerInvocationException(e);
        } catch (InvocationTargetException e) {
            Throwable cause = e.getCause();
            throw (cause instanceof RuntimeException && !(cause instanceof MissingMethodException))
?
                    (RuntimeException) cause : new InvokerInvocationException(e);
        }
    }

    public final Method setAccessible() {
        try {
            AccessPermissionChecker.checkAccessPermission(cachedMethod);
        }
        catch (AccessControlException ex) {
            throw new IllegalArgumentException("Illegal access to method" + cachedMethod.getName());
        }
//        if (queuedToCompile.compareAndSet(false,true)) {
//            if (isCompilable())
//              CompileThread.addMethod(this);
//        }
        return cachedMethod;
    }

    public Method getCachedMethod() {
        try {
            AccessPermissionChecker.checkAccessPermission(cachedMethod);
        }
        catch (AccessControlException ex) {
            throw new IllegalArgumentException("Illegal access to method" + cachedMethod.getName());
        }
        return cachedMethod;
    }
}

{code}

In order to apply the patch in Freeplane I created a separate project https://github.com/dpolivaev/securegroovy
which generates the patch at runtime using bytebuddy. 

I would appreciate if you could integrate the patch in groovy code.

There is one subtle issue with AccessPermissionChecker: as you see it allows access to all
protected methods except for ClassLoader without any further checks.
But for the protected methods from ClassLoader is makes one additional check: Otherwise a
script could access a class loader by getClass().getClassLoader() and misuse its defineClass
method to let malicious code appear trusted. 


> Groovy scripts can disable java security manager and escape sandbox
> -------------------------------------------------------------------
>
>                 Key: GROOVY-8163
>                 URL: https://issues.apache.org/jira/browse/GROOVY-8163
>             Project: Groovy
>          Issue Type: Bug
>    Affects Versions: 2.5.0-alpha-1, 2.4.9, 2.4.10
>            Reporter: Dimitry Polivaev
>
> Consider following test
> {code}
> package groovytest;
> import groovy.util.Eval;
> import org.junit.*;
> import java.net.URL;
> import java.security.AccessController;
> import java.security.PrivilegedAction;
> public class GroovySecurityTest {
> 	public static final String RESTRICTED_PERMISSIONS_FOR_SCRIPT_ONLY_POLICY = "/restrictedPermissionsForScriptOnlyPolicy.txt";
> 	public static final String POLICY = RESTRICTED_PERMISSIONS_FOR_SCRIPT_ONLY_POLICY;
> 	@BeforeClass
> 	public static void setPolicy() throws Exception {
> 		final String dirTest = GroovySecurityTest.class.getProtectionDomain().getCodeSource().getLocation().toString();
> 		final String dirGroovy = Eval.class.getProtectionDomain().getCodeSource().getLocation().toString();
> 		System.setProperty("dir.test",dirTest + "-");
> 		System.setProperty("dir.groovy",dirGroovy);
> 		final URL policy = GroovySecurityTest.class.getResource(POLICY);
> 		System.setProperty("java.security.policy", policy.toString());
> 	}
> 	
> 	
> 	@Before
> 	public void setSecurityManager() throws Exception {
> 			System.setSecurityManager(new SecurityManager());
> 	}
> 	@After
> 	public void removeSecurityManager() throws Exception {
> 		AccessController.doPrivileged(new PrivilegedAction<Void>() {
> 			@Override
> 			public Void run() {
> 				System.setSecurityManager(null);
> 				return null;
> 			}
> 		});
> 	}
> 	@Test
> 	public void doesNotChangeScriptPermissionsUsungPrivateFieldAccess() throws Exception
{
> 		try {
> 			AccessController.doPrivileged(new PrivilegedAction<Void>() {
>                 @Override
>                 public Void run() {
>                     Eval.me("getClass().protectionDomain0.hasAllPerm = true;"
>                                     + "System.setSecurityManager(null);"
>                                     + "1");
>                     return null;
>                 }
>             });
> 		} catch (Exception e) {
> 		}
> 		Assert.assertNotNull(System.getSecurityManager());
> 	}
> }
> {code}
> with following policy file restrictedPermissionsForScriptOnlyPolicy.txt
> {code}
> grant codeBase "${dir.test}" {
> 	permission java.security.AllPermission;
> };
> grant codeBase "${dir.groovy}" {
> 	permission java.security.AllPermission;
> };
> grant {
> };
> {code}
> It fails: security manager is not set any more when the test assertion is checked.
> It happens because CachedField from org.codehaus.groovy.reflection is created withing
trusted code base (groovy jar) and gives access to the field to untrusted scripts without
any security checks. The same problem relates to CachedMethod which would allow any script
to access protected method java.lang.ClassLoader#defineClass(java.lang.String, byte[], int,
int, java.security.ProtectionDomain) that can be misused to manipulate code sources of classes
loaded from script to give them all permissions.
> It also appears that if I remove permissions from groovy.jar using more restrictive policy
using following policy file restrictedPermissionsPolicy.txt
> {code}
> grant  codeBase "${dir.test}" {
>     permission java.lang.RuntimePermission "*";
>     permission java.security.SecurityPermission "*";
>      permission java.io.FilePermission "<<ALL FILES>>", "read";
>     permission java.util.PropertyPermission "*", "read";
>     permission groovy.security.GroovyCodeSourcePermission "*";
> };
> grant  codeBase "${dir.groovy}" {
>     permission java.lang.RuntimePermission "*";
>     permission java.security.SecurityPermission "*";
>     permission java.io.FilePermission "<<ALL FILES>>", "read";
>     permission java.util.PropertyPermission "*", "read";
>     permission groovy.security.GroovyCodeSourcePermission "*";
> };
> grant {
>     permission java.lang.RuntimePermission "accessDeclaredMembers";
> };
> {code}
> it has a consequence that groovy can not access even some public methods on bean properties
as shown in the following test
> {code}
> package groovytest;
> import groovy.util.Eval;
> import org.junit.*;
> import java.net.URL;
> import java.security.AccessController;
> import java.security.PrivilegedAction;
> public class GroovyBeanTest {
> 	public static final String RESTRICTED_PERMISSIONS_POLICY = "/restrictedPermissionsPolicy.txt";
> 	public static final String POLICY = RESTRICTED_PERMISSIONS_POLICY;
> 	@BeforeClass
> 	public static void setPolicy() throws Exception {
> 		final String dirTest = GroovyBeanTest.class.getProtectionDomain().getCodeSource().getLocation().toString();
> 		final String dirGroovy = Eval.class.getProtectionDomain().getCodeSource().getLocation().toString();
> 		System.setProperty("dir.test",dirTest + "-");
> 		System.setProperty("dir.groovy",dirGroovy);
> 		final URL policy = GroovyBeanTest.class.getResource(POLICY);
> 		System.setProperty("java.security.policy", policy.toString());
> 	}
> 	
> 	
> 	@Before
> 	public void setSecurityManager() throws Exception {
> 			System.setSecurityManager(new SecurityManager());
> 	}
> 	@After
> 	public void removeSecurityManager() throws Exception {
> 		AccessController.doPrivileged(new PrivilegedAction<Void>() {
> 			@Override
> 			public Void run() {
> 				System.setSecurityManager(null);
> 				return null;
> 			}
> 		});
> 	}
> 	
> 	public interface BeanInterface{
> 		public String getName();
> 	}
> 	private class Bean implements BeanInterface{
> 		private String _name = "bean";
> 		public String getName(){
> 			return _name;
> 		}
> 	}
> 	@Test
> 	public void returnsBeanPropertyValue() throws Exception {
> 		AccessController.doPrivileged(new PrivilegedAction<Void>() {
> 			@Override
> 			public Void run() {
> 				Assert.assertEquals("bean", Eval.x(new Bean(), "x.name"));
> 				return null;
> 			}
> 		});
> 	}
> 	static class BeanClass {
> 		private String name = "bean";
> 	}
> 	@Test
> 	public void returnsBeanClassName() throws Exception {
> 		AccessController.doPrivileged(new PrivilegedAction<Void>() {
> 			@Override
> 			public Void run() {
> 				Assert.assertEquals(BeanClass.class.getName(), Eval.x(new BeanClass(), "x.class.name"));
> 				return null;
> 			}
> 		});
> 	}
> }
> {code}
> The both tests fail as the bean properties can not be found by groovy.
> It turned out that I can not run the both tests in one process, make sure you run them
separately.
> In order to fix this issue for my open source project Freeplane I have to patch groovy
code. It turned out to be possible. 
> So I want to share the fix with you and ask you to integrate it.
> The fix is based on groovy version 2.4.9 and I think that it can be applied to any Groovy
version.
> You only need to check for access permissions at the relevant places to make sure that
they are not leaked from groovy (which needs them to work properly) to the untrusted script
> {code}
> 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 groovy.lang.GroovyObject;
> class AccessPermissionChecker {
> 	private static final ReflectPermission REFLECT_PERMISSION = new ReflectPermission("suppressAccessChecks");
> 	static private void checkAccessPermission(Class<?> declaringClass, final int modifiers,
boolean isAccessible) {
> 		final SecurityManager securityManager = System.getSecurityManager();
> 		if (isAccessible && securityManager != null) {
> 				if ((modifiers & (Modifier.PUBLIC | Modifier.PROTECTED)) == 0
> 						&& !GroovyObject.class.isAssignableFrom(declaringClass)) {
>                         securityManager.checkPermission(REFLECT_PERMISSION);
>                 }
>                 else if ((modifiers & (Modifier.PUBLIC)) == 0
> 					&& declaringClass.equals(ClassLoader.class)){
> 					securityManager.checkCreateClassLoader();
> 				}
> 		}
> 	}
> 	static public void checkAccessPermission(Method method) {
> 		checkAccessPermission(method.getDeclaringClass(), method.getModifiers(), method.isAccessible()
> 		);
> 	}
> 	public static void checkAccessPermission(Field field) {
> 		checkAccessPermission(field.getDeclaringClass(), field.getModifiers(), field.isAccessible()
> 		);
> 	}
> }
> {code}
> patching your classes as follows:
> CachedField.java:
> {code}
>     /**
>      * @return the property of the given object
>      * @throws RuntimeException if the property could not be evaluated
>      */
>     public Object getProperty(final Object object) {
>         try {
>             AccessPermissionChecker.checkAccessPermission(field);
>         }
>         catch (AccessControlException ex) {
>             throw new IllegalArgumentException("Illegal access to field" + " " + field.getName());
>         }
>         try {
>             return field.get(object);
>         } catch (IllegalAccessException e) {
>             throw new GroovyRuntimeException("Cannot get the property '" + name + "'.",
e);
>         }
>     }
>     /**
>      * Sets the property on the given object to the new value
>      *
>      * @param object on which to set the property
>      * @param newValue the new value of the property
>      * @throws RuntimeException if the property could not be set
>      */
>     public void setProperty(final Object object, Object newValue) {
>         try {
>             AccessPermissionChecker.checkAccessPermission(field);
>         }
>         catch (AccessControlException ex) {
>             throw new IllegalArgumentException("Illegal access to field" + " " + field.getName());
>         }
>         final Object goalValue = DefaultTypeTransformation.castToType(newValue, field.getType());
>         if (isFinal()) {
>             throw new GroovyRuntimeException("Cannot set the property '" + name + "'
because the backing field is final.");
>         }
>         try {
>             field.set(object, goalValue);
>         } catch (IllegalAccessException ex) {
>             throw new GroovyRuntimeException("Cannot set the property '" + name + "'.",
ex);
>         }
>     }
> {code}
> CachedMethod.java:
> {code}
>     public final Object invoke(Object object, Object[] arguments) {
>         try {
>             AccessPermissionChecker.checkAccessPermission(cachedMethod);
>         }
>         catch (AccessControlException ex) {
>             throw new InvokerInvocationException(new IllegalArgumentException("Illegal
access to method" + cachedMethod.getName()));
>         }
>         try {
>             return cachedMethod.invoke(object, arguments);
>         } catch (IllegalArgumentException e) {
>             throw new InvokerInvocationException(e);
>         } catch (IllegalAccessException e) {
>             throw new InvokerInvocationException(e);
>         } catch (InvocationTargetException e) {
>             Throwable cause = e.getCause();
>             throw (cause instanceof RuntimeException && !(cause instanceof MissingMethodException))
?
>                     (RuntimeException) cause : new InvokerInvocationException(e);
>         }
>     }
>     public final Method setAccessible() {
>         try {
>             AccessPermissionChecker.checkAccessPermission(cachedMethod);
>         }
>         catch (AccessControlException ex) {
>             throw new IllegalArgumentException("Illegal access to method" + cachedMethod.getName());
>         }
> //        if (queuedToCompile.compareAndSet(false,true)) {
> //            if (isCompilable())
> //              CompileThread.addMethod(this);
> //        }
>         return cachedMethod;
>     }
>     public Method getCachedMethod() {
>         try {
>             AccessPermissionChecker.checkAccessPermission(cachedMethod);
>         }
>         catch (AccessControlException ex) {
>             throw new IllegalArgumentException("Illegal access to method" + cachedMethod.getName());
>         }
>         return cachedMethod;
>     }
> }
> {code}
> In order to apply the patch in Freeplane I created a separate project https://github.com/dpolivaev/securegroovy
which generates the patch at runtime using bytebuddy. 
> I would appreciate if you could integrate the patch in groovy code.
> There is one subtle issue with AccessPermissionChecker: as you see it allows access to
all protected methods except for ClassLoader without any further checks.
> But for the protected methods from ClassLoader is makes one additional check: Otherwise
a script could access a class loader by getClass().getClassLoader() and misuse its defineClass
method to let malicious code appear trusted. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message