You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "Paul King (JIRA)" <ji...@apache.org> on 2018/03/06 23:26:11 UTC
[jira] [Closed] (GROOVY-8163) Groovy scripts can disable java
security manager and escape sandbox
[ https://issues.apache.org/jira/browse/GROOVY-8163?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Paul King closed GROOVY-8163.
-----------------------------
> 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
> Assignee: John Wagenleitner
> Priority: Major
> Fix For: 2.5.0-beta-2
>
>
> 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
(v7.6.3#76005)