Add sandboxing for GString-based method invocation

elastic · Jun 25, 2014 · 47856ec · 47856ec
1 parent 342563a
commit 47856ec
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 1 deletion.
diff --git a/src/main/java/org/elasticsearch/script/groovy/GroovySandboxExpressionChecker.java b/src/main/java/org/elasticsearch/script/groovy/GroovySandboxExpressionChecker.java
@@ -23,6 +23,7 @@
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
 import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.GStringExpression;
 import org.codehaus.groovy.ast.expr.MethodCallExpression;
 import org.codehaus.groovy.control.customizers.SecureASTCustomizer;
 import org.elasticsearch.common.settings.Settings;
@@ -115,7 +116,11 @@ public GroovySandboxExpressionChecker(Settings settings) {
     public boolean isAuthorized(Expression expression) {
         if (expression instanceof MethodCallExpression) {
             MethodCallExpression mce = (MethodCallExpression) expression;
-            if (methodBlacklist.contains(mce.getMethodAsString())) {
+            String methodName = mce.getMethodAsString();
+            if (methodBlacklist.contains(methodName)) {
+                return false;
+            } else if (methodName == null && mce.getMethod() instanceof GStringExpression) {
+                // We do not allow GStrings for method invocation, they are a security risk
                 return false;
             }
         } else if (expression instanceof ConstructorCallExpression) {

diff --git a/src/test/java/org/elasticsearch/script/GroovySandboxScriptTests.java b/src/test/java/org/elasticsearch/script/GroovySandboxScriptTests.java
@@ -55,6 +55,10 @@ public void testSandboxedGroovyScript() {
         testFailure("d = new DateTime(); d.getClass().getDeclaredMethod(\\\"plus\\\").setAccessible(true)",
                 "Expression [MethodCallExpression] is not allowed: d.getClass()");
 
+        testFailure("d = new DateTime(); d.\\\"${'get' + 'Class'}\\\"()." +
+                        "\\\"${'getDeclared' + 'Method'}\\\"(\\\"now\\\").\\\"${'set' + 'Accessible'}\\\"(false)",
+                "Expression [MethodCallExpression] is not allowed: d.$(get + Class)().$(getDeclared + Method)(now).$(set + Accessible)(false)");
+
         testFailure("Class.forName(\\\"DateTime\\\").getDeclaredMethod(\\\"plus\\\").setAccessible(true)",
                 "Method calls not allowed on [java.lang.Class]");
 
@@ -79,6 +83,9 @@ public void testSandboxedGroovyScript() {
                 "Importing [java.util.concurrent.ThreadPoolExecutor] is not allowed");
 
         testFailure("s = new java.net.URL();", "Expression [ConstructorCallExpression] is not allowed: new java.net.URL()");
+
+        testFailure("def methodName = 'ex'; Runtime.\\\"${'get' + 'Runtime'}\\\"().\\\"${methodName}ec\\\"(\\\"touch /tmp/gotcha2\\\")",
+                "Expression [MethodCallExpression] is not allowed: java.lang.Runtime.$(get + Runtime)().$methodNameec(touch /tmp/gotcha2)");
     }
 
     public void testSuccess(String script) {