Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #13 from jenkinsci/JENKINS-28586
[FIXED JENKINS-28586] handle method calls/property accesses via closure
  • Loading branch information
jglick committed Jun 1, 2015
2 parents c6d43e7 + b924d4a commit 5024073
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 21 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -39,7 +39,7 @@
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>groovy-sandbox</artifactId>
<version>1.8</version>
<version>1.9</version>
<exclusions>
<exclusion>
<groupId>org.codehaus.groovy</groupId>
Expand Down
Expand Up @@ -27,6 +27,15 @@ method java.util.concurrent.Callable call
# Groovy:
method groovy.lang.Script getBinding
method groovy.lang.Closure call java.lang.Object
staticField groovy.lang.Closure OWNER_FIRST
staticField groovy.lang.Closure DELEGATE_FIRST
staticField groovy.lang.Closure OWNER_ONLY
staticField groovy.lang.Closure DELEGATE_ONLY
staticField groovy.lang.Closure TO_SELF
method groovy.lang.Closure getResolveStrategy
method groovy.lang.Closure setResolveStrategy int
method groovy.lang.Closure getDelegate
method groovy.lang.Closure setDelegate java.lang.Object
staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter compareEqual java.lang.Object java.lang.Object
staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter compareNotEqual java.lang.Object java.lang.Object
staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter findRegex java.lang.Object java.lang.Object
Expand Down
Expand Up @@ -25,7 +25,7 @@
package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy;

import groovy.json.JsonBuilder;
import groovy.lang.Closure;
import groovy.json.JsonDelegate;
import groovy.lang.GString;
import groovy.lang.GroovyObject;
import groovy.lang.GroovyObjectSupport;
Expand All @@ -43,6 +43,8 @@
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.Callable;

import org.apache.commons.lang.StringUtils;
import org.codehaus.groovy.control.CompilerConfiguration;
import org.codehaus.groovy.runtime.GStringImpl;
import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException;
Expand Down Expand Up @@ -99,10 +101,8 @@ public class SandboxInterceptorTest {
public boolean permitsMethod(Method method, Object receiver, Object[] args) {
if (method.getName().equals("invokeMethod") && receiver instanceof JsonBuilder)
return true;
if (method.getName().equals("invokeMethod") && receiver instanceof Closure) {
Object d = ((Closure) receiver).getDelegate();
return d.getClass().getName().equals("groovy.json.JsonDelegate");
}
if (method.getName().equals("invokeMethod") && receiver instanceof JsonDelegate)
return true;
if (method.getName().equals("toString") && receiver instanceof JsonBuilder)
return true;
return false;
Expand Down Expand Up @@ -250,7 +250,7 @@ public static final class Special {
assertRejected(new ProxyWhitelist(), "staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toInteger java.lang.String", "'123'.toInteger();");
assertEvaluate(new StaticWhitelist("staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toInteger java.lang.String"), 123, "'123'.toInteger();");
assertEvaluate(new StaticWhitelist("staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods collect java.lang.Object groovy.lang.Closure"), Arrays.asList(1, 4, 9), "([1, 2, 3] as int[]).collect({x -> x * x})");
/* TODO No such property: it for class: Script1:
/* TODO No such property: it for class: groovy.lang.Binding:
assertEvaluate(new StaticWhitelist("staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods collect java.lang.Object groovy.lang.Closure"), Arrays.asList(1, 4, 9), "([1, 2, 3] as int[]).collect({it * it})");
*/
}
Expand Down Expand Up @@ -300,25 +300,62 @@ private Unsafe() {}
}
}

@Test public void closures() throws Exception {
// TODO https://github.com/kohsuke/groovy-sandbox/issues/11 would like that to be rejecting method java.lang.Throwable getMessage
assertRejected(
new StaticWhitelist(
"method java.util.concurrent.Callable call",
"field groovy.lang.Closure delegate",
"new java.lang.Exception java.lang.String"),
"method groovy.lang.GroovyObject getProperty java.lang.String",
/**
* Tests the method invocation / property access through closures.
*
* <p>
* Groovy closures act as a proxy when it comes to property/method access. Based on the configuration, it can
* access those from some combination of owner/delegate. As this is an important building block for custom DSL,
* script-security understands this logic and checks access at the actual target of the proxy, so that Closures
* can be used safely.
*/
@Test public void closureDelegate() throws Exception {
ProxyWhitelist rules = new ProxyWhitelist(new GenericWhitelist(), new StaticWhitelist("new java.lang.Exception java.lang.String"));
assertRejected(rules,
"method java.lang.Throwable getMessage",
"{-> delegate = new Exception('oops'); message}()"
);
// TODO similarly this would preferably be rejecting method java.lang.Throwable printStackTrace
assertRejected(
new StaticWhitelist(
"method java.util.concurrent.Callable call",
"field groovy.lang.Closure delegate",
"new java.lang.Exception java.lang.String"),
"method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object",
rules,
"method java.lang.Throwable printStackTrace",
"{-> delegate = new Exception('oops'); printStackTrace()}()"
);

rules = new ProxyWhitelist(new GenericWhitelist(), new StaticWhitelist("new java.awt.Point"));
{ // method access
assertEvaluate(rules, 3,
StringUtils.join(Arrays.asList(
"class Dummy { def getX() { return 3; } }",
"def c = { -> getX() };",
"c.resolveStrategy = Closure.DELEGATE_ONLY;",
"c.delegate = new Dummy();",
"return c();"
), "\n"));
assertRejected(rules, "method java.awt.geom.Point2D getX",
StringUtils.join(Arrays.asList(
"def c = { -> getX() };",
"c.resolveStrategy = Closure.DELEGATE_ONLY;",
"c.delegate = new java.awt.Point();",
"return c();"
), "\n"));
}
{// property access
assertEvaluate(rules, 3,
StringUtils.join(Arrays.asList(
"class Dummy { def getX() { return 3; } }",
"def c = { -> x };",
"c.resolveStrategy = Closure.DELEGATE_ONLY;",
"c.delegate = new Dummy();",
"return c();"
), "\n"));
assertRejected(rules, "field java.awt.Point x",
StringUtils.join(Arrays.asList(
"def c = { -> x };",
"c.resolveStrategy = Closure.DELEGATE_ONLY;",
"c.delegate = new java.awt.Point();",
"return c();"
), "\n"));
}
}

@Test public void templates() throws Exception {
Expand Down

0 comments on commit 5024073

Please sign in to comment.