Skip to content

Commit

Permalink
[FIXED JENKINS-34739] Properly handle varargs calls.
Browse files Browse the repository at this point in the history
  • Loading branch information
@jglick
jglick committed May 11, 2016
1 parent 57d6eb3 commit 8acc84d
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 23 deletions.
45 changes: 24 additions & 21 deletions ...main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java
View file
Expand Up @@ -26,6 +26,7 @@

import com.google.common.primitives.Primitives;
import groovy.lang.GString;
import java.lang.reflect.Array;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
Expand All @@ -43,7 +44,27 @@
*/
class GroovyCallSiteSelector {

private static boolean matches(@Nonnull Class<?>[] parameterTypes, @Nonnull Object[] parameters) {
private static boolean matches(@Nonnull Class<?>[] parameterTypes, @Nonnull Object[] parameters, boolean varargs) {
if (varargs) {
int fixedLen = parameterTypes.length - 1;
Class<?> componentType = parameterTypes[fixedLen].getComponentType();
assert componentType != null;
int arrayLength = parameters.length - fixedLen;
if (arrayLength >= 0) {
if (arrayLength == 1 && parameterTypes[fixedLen].isInstance(parameters[fixedLen])) {
// not a varargs call
} else {
Object array = Array.newInstance(componentType, arrayLength);
for (int i = 0; i < arrayLength; i++) {
Array.set(array, i, parameters[fixedLen + i]);
}
Object[] parameters2 = new Object[fixedLen + 1];
System.arraycopy(parameters, 0, parameters2, 0, fixedLen);
parameters2[fixedLen] = array;
parameters = parameters2;
}
}
}
if (parameters.length != parameterTypes.length) {
return false;
}
Expand Down Expand Up @@ -128,7 +149,7 @@ private static boolean isInstancePrimitive(@Nonnull Class<?> type, @Nonnull Obje

public static @CheckForNull Constructor<?> constructor(@Nonnull Class<?> receiver, @Nonnull Object[] args) {
for (Constructor<?> c : receiver.getDeclaredConstructors()) {
if (matches(c.getParameterTypes(), args)) {
if (matches(c.getParameterTypes(), args, c.isVarArgs())) {
return c;
}
}
Expand All @@ -140,28 +161,10 @@ private static boolean isInstancePrimitive(@Nonnull Class<?> type, @Nonnull Obje
return findMatchingMethod(receiver, method, args);
}

private static boolean isEnumInitializer(Method m) {
if (!m.getDeclaringClass().isEnum()) {
return false;
}
if (!m.isSynthetic()) {
return false;
}
final Class<?>[] parameterTypes = m.getParameterTypes();
if (parameterTypes.length != 1) {
return false;
}
final Class<?> type = parameterTypes[0];
if (!type.isArray()) {
return false;
}
return type.getComponentType().equals(Object.class);
}

private static Method findMatchingMethod(Class<?> receiver, String method, Object[] args) {
Method candidate = null;
for (Method m : receiver.getDeclaredMethods()) {
if (m.getName().equals(method) && (matches(m.getParameterTypes(), args) || isEnumInitializer(m))) {
if (m.getName().equals(method) && (matches(m.getParameterTypes(), args, m.isVarArgs()))) {
if (candidate == null || isMoreSpecific(m, candidate)) {
candidate = m;
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/generic-whitelist
View file
Expand Up @@ -23,10 +23,13 @@ staticField groovy.lang.Closure OWNER_FIRST
staticField groovy.lang.Closure OWNER_ONLY
staticField groovy.lang.Closure TO_SELF
method groovy.lang.Closure call java.lang.Object
method groovy.lang.Closure call java.lang.Object[]
method groovy.lang.Closure curry java.lang.Object
method groovy.lang.Closure curry java.lang.Object[]
method groovy.lang.Closure getDelegate
method groovy.lang.Closure getResolveStrategy
method groovy.lang.Closure ncurry int java.lang.Object
method groovy.lang.Closure ncurry int java.lang.Object[]
method groovy.lang.Closure setDelegate java.lang.Object
method groovy.lang.Closure setResolveStrategy int
method groovy.lang.Script getBinding
Expand Down
44 changes: 42 additions & 2 deletions ...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java
View file
Expand Up @@ -69,7 +69,6 @@
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.ProxyWhitelist;
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.StaticWhitelist;
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
import org.junit.AssumptionViolatedException;
import org.junit.Ignore;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
Expand Down Expand Up @@ -425,7 +424,7 @@ private Unsafe() {}
new GroovyShell().evaluate("String.metaClass.getAnswer = {-> return 42}"); // privileged operation
assertEvaluate(new StaticWhitelist(), 42, "'existence'.getAnswer()");
assertEvaluate(new StaticWhitelist(), 42, "'existence'.answer");
assertRejected(new StaticWhitelist("method groovy.lang.Closure call java.lang.Object"), "staticMethod java.lang.System exit int", "def c = System.&exit; c(1)");
assertRejected(new GenericWhitelist(), "staticMethod java.lang.System exit int", "def c = System.&exit; c(1)");
}

@Issue("JENKINS-28277")
Expand All @@ -435,6 +434,47 @@ private Unsafe() {}
assertEvaluate(new GenericWhitelist(), 'h', "def charAt = {str, idx -> str.charAt(idx)}; def firstChar = charAt.ncurry(1, 0); firstChar 'hello'");
}

@Issue("JENKINS-34739")
@Test public void varargs() throws Exception {
// Control cases:
ProxyWhitelist wl = new ProxyWhitelist(new GenericWhitelist(), new AnnotatedWhitelist());
assertEvaluate(wl, 0, "class UsesVarargs {static int len(String... vals) {vals.length}}; UsesVarargs.len(new String[0])");
assertEvaluate(wl, 3, "class UsesVarargs {static int len(String... vals) {vals.length}}; UsesVarargs.len(['one', 'two', 'three'] as String[])");
String uv = UsesVarargs.class.getName();
assertEvaluate(wl, 0, uv + ".len(new String[0])");
assertEvaluate(wl, 3, uv + ".len(['one', 'two', 'three'] as String[])");
assertEvaluate(wl, 3, uv + ".xlen(3, new String[0])");
assertEvaluate(wl, 6, uv + ".xlen(3, ['one', 'two', 'three'] as String[])");
assertEvaluate(wl, "one,two,three", uv + ".join(',', ['one', 'two', 'three'] as String[])");
assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode(new String[0])");
assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode(['one', 'two', 'three'] as String[])");
// Test cases:
assertEvaluate(wl, 0, "class UsesVarargs {static int len(String... vals) {vals.length}}; UsesVarargs.len()");
assertEvaluate(wl, 3, "class UsesVarargs {static int len(String... vals) {vals.length}}; UsesVarargs.len('one', 'two', 'three')");
assertEvaluate(wl, 0, uv + ".len()");
assertEvaluate(wl, 3, uv + ".xlen(3)");
assertEvaluate(wl, 3, uv + ".len('one', 'two', 'three')");
assertEvaluate(wl, 6, uv + ".xlen(3, 'one', 'two', 'three')");
assertEvaluate(wl, "one,two,three", uv + ".join(',', 'one', 'two', 'three')");
assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode()");
assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode('one', 'two', 'three')");
}
public static class UsesVarargs {
@Whitelisted
public static int len(String... vals) {
return vals.length;
}
@Whitelisted
public static int xlen(int x, String... vals) {
return x + vals.length;
}
@Whitelisted
public static String join(String sep, String... vals) {
return StringUtils.join(vals, sep);
}
public static void explode(String... vals) {}
}

@Test public void templates() throws Exception {
final GroovyShell shell = new GroovyShell(GroovySandbox.createSecureCompilerConfiguration());
final Template t = new SimpleTemplateEngine(shell).createTemplate("goodbye <%= aspect.toLowerCase() %> world");
Expand Down

0 comments on commit 8acc84d

Please sign in to comment.