-
Type:
Bug
-
Resolution: Fixed
-
Priority:
Critical
-
Component/s: script-security-plugin
GroovyCallSiteSelector.isMoreSpecific is supposed to select the most applicable method for a given call. Yet this only applies to Method selection. Constructor selection just picks the "first" matching constructor, which is nondeterministic, so that for example a call to new hudson.EnvVars() given the current whitelist entry new hudson.EnvVars may or may not succeed, depending on whether you use the plugin as is or with the following applied:
diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java index cf253de..62e5e79 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java @@ -30,11 +30,13 @@ import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.ClassUtils; /** @@ -159,7 +161,9 @@ class GroovyCallSiteSelector { } public static @CheckForNull Constructor<?> constructor(@Nonnull Class<?> receiver, @Nonnull Object[] args) { - for (Constructor<?> c : receiver.getDeclaredConstructors()) { + Constructor<?>[] constructors = receiver.getDeclaredConstructors(); + ArrayUtils.reverse(constructors); + for (Constructor<?> c : constructors) { if (matches(c.getParameterTypes(), args, c.isVarArgs())) { return c; } @@ -169,7 +173,7 @@ class GroovyCallSiteSelector { // Also note that this logic is derived from how Groovy itself decides to use the magic Map constructor, at // MetaClassImpl#invokeConstructor(Class, Object[]). if (args.length == 1 && args[0] instanceof Map) { - for (Constructor<?> c : receiver.getDeclaredConstructors()) { + for (Constructor<?> c : constructors) { if (c.getParameterTypes().length == 0 && !c.isVarArgs()) { return c; }
In this case we would want the non-varargs overload (the one currently whitelisted) to be preferred.
- relates to
-
-
- Closed
-
- links to
