From 47856ec4cdbe5fdd33cc1927af832c130c4ae06b Mon Sep 17 00:00:00 2001
From: Lee Hinman <lee@writequit.org>
Date: Mon, 23 Jun 2014 15:47:59 +0200
Subject: [PATCH] Add sandboxing for GString-based method invocation

---
 .../script/groovy/GroovySandboxExpressionChecker.java      | 7 ++++++-
 .../org/elasticsearch/script/GroovySandboxScriptTests.java | 7 +++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/elasticsearch/script/groovy/GroovySandboxExpressionChecker.java b/src/main/java/org/elasticsearch/script/groovy/GroovySandboxExpressionChecker.java
index 1fdd6965df1..1ed86fe9959 100644
--- 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 com.google.common.collect.ImmutableSet;
 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 class GroovySandboxExpressionChecker implements SecureASTCustomizer.Expre
     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
index 1ffae32cffd..68a90420a8b 100644
--- a/src/test/java/org/elasticsearch/script/GroovySandboxScriptTests.java
+++ b/src/test/java/org/elasticsearch/script/GroovySandboxScriptTests.java
@@ -55,6 +55,10 @@ public class GroovySandboxScriptTests extends ElasticsearchIntegrationTest {
         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 class GroovySandboxScriptTests extends ElasticsearchIntegrationTest {
                 "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) {