Skip to content

Commit

Permalink
[SECURITY-258] Trap field access and array index calls.
Browse files Browse the repository at this point in the history
  • Loading branch information
jglick committed Mar 28, 2016
1 parent 142f50d commit e7d3bc9
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -348,13 +348,87 @@ private interface Rejector {
@Nonnull RejectedAccessException reject();
}

// TODO consider whether it is useful to override onGet/SetArray/Attribute
@Override public Object onGetAttribute(Invoker invoker, Object receiver, String attribute) throws Throwable {
Field field = GroovyCallSiteSelector.field(receiver, attribute);
if (field == null) {
throw unclassifiedField(receiver, attribute);
} else if (whitelist.permitsFieldGet(field, receiver)) {
return super.onGetAttribute(invoker, receiver, attribute);
} else {
throw StaticWhitelist.rejectField(field);
}
}

@Override public Object onSetAttribute(Invoker invoker, Object receiver, String attribute, Object value) throws Throwable {
Field field = GroovyCallSiteSelector.field(receiver, attribute);
if (field == null) {
throw unclassifiedField(receiver, attribute);
} else if (whitelist.permitsFieldSet(field, receiver, value)) {
return super.onSetAttribute(invoker, receiver, attribute, value);
} else {
throw StaticWhitelist.rejectField(field);
}
}

@Override public Object onGetArray(Invoker invoker, Object receiver, Object index) throws Throwable {
if (receiver.getClass().isArray() && index instanceof Integer) {
return super.onGetArray(invoker, receiver, index);
}
Object[] args = new Object[] {index};
Method method = GroovyCallSiteSelector.method(receiver, "getAt", args);
if (method != null) {
if (whitelist.permitsMethod(method, receiver, args)) {
return super.onGetArray(invoker, receiver, index);
} else {
throw StaticWhitelist.rejectMethod(method);
}
}
args = new Object[] {receiver, index};
for (Class<?> dgm : DGM_CLASSES) {
method = GroovyCallSiteSelector.staticMethod(dgm, "getAt", args);
if (method != null) {
if (whitelist.permitsStaticMethod(method, args)) {
return super.onGetArray(invoker, receiver, index);
} else {
throw StaticWhitelist.rejectStaticMethod(method);
}
}
}
throw new RejectedAccessException("unclassified getAt method " + EnumeratingWhitelist.getName(receiver) + "[" + EnumeratingWhitelist.getName(index) + "]");
}

@Override public Object onSetArray(Invoker invoker, Object receiver, Object index, Object value) throws Throwable {
if (receiver.getClass().isArray() && index instanceof Integer) {
return super.onSetArray(invoker, receiver, index, value);
}
Object[] args = new Object[] {index, value};
Method method = GroovyCallSiteSelector.method(receiver, "putAt", args);
if (method != null) {
if (whitelist.permitsMethod(method, receiver, args)) {
return super.onSetArray(invoker, receiver, index, value);
} else {
throw StaticWhitelist.rejectMethod(method);
}
}
args = new Object[] {receiver, index, value};
for (Class<?> dgm : DGM_CLASSES) {
method = GroovyCallSiteSelector.staticMethod(dgm, "putAt", args);
if (method != null) {
if (whitelist.permitsStaticMethod(method, args)) {
return super.onSetArray(invoker, receiver, index, value);
} else {
throw StaticWhitelist.rejectStaticMethod(method);
}
}
}
throw new RejectedAccessException("unclassified putAt method " + EnumeratingWhitelist.getName(receiver) + "[" + EnumeratingWhitelist.getName(index) + "]=" + EnumeratingWhitelist.getName(value));
}

private static String printArgumentTypes(Object[] args) {
StringBuilder b = new StringBuilder();
for (Object arg : args) {
b.append(' ');
b.append(arg == null ? "null" : EnumeratingWhitelist.getName(arg.getClass()));
b.append(EnumeratingWhitelist.getName(arg));
}
return b.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import org.apache.commons.lang.ClassUtils;
import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist;

Expand Down Expand Up @@ -112,7 +114,7 @@ public abstract class EnumeratingWhitelist extends Whitelist {
return false;
}

public static String getName(Class<?> c) {
public static @Nonnull String getName(@Nonnull Class<?> c) {
Class<?> e = c.getComponentType();
if (e == null) {
return c.getName();
Expand All @@ -121,6 +123,10 @@ public static String getName(Class<?> c) {
}
}

public static @Nonnull String getName(@CheckForNull Object o) {
return o == null ? "null" : getName(o.getClass());
}

private static String[] argumentTypes(Class<?>[] argumentTypes) {
String[] s = new String[argumentTypes.length];
for (int i = 0; i < argumentTypes.length; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods eachLine java.lang
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods eachLine java.lang.CharSequence int groovy.lang.Closure
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods eachLine java.lang.String groovy.lang.Closure
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods eachLine java.lang.String int groovy.lang.Closure
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods getAt java.util.List int
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods getAt java.util.Map java.lang.Object
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods getAt java.util.regex.Matcher int
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods getChars java.lang.String
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods isCase java.util.Collection java.lang.Object
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods isNumber java.lang.String
Expand All @@ -134,6 +137,8 @@ staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods plus java.lang.Cha
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods plus java.lang.Number java.lang.String
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods plus java.lang.String java.lang.Object
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods plus java.lang.StringBuffer java.lang.String
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods putAt java.util.List int java.lang.Object
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods putAt java.util.Map java.lang.Object java.lang.Object
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods size boolean[]
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods size byte[]
staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods size char[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,10 @@ public boolean permitsMethod(Method method, Object receiver, Object[] args) {
assertEvaluate(new StaticWhitelist("new " + clazz, "method " + clazz + " getProp5"), "DEFAULT", "new " + clazz + "().prop5");
assertRejected(new StaticWhitelist("new " + clazz, "method " + clazz + " getProp5"), "method " + clazz + " setProp5 java.lang.String", "def c = new " + clazz + "(); c.prop5 = 'EDITED'; c.prop5");
assertEvaluate(new StaticWhitelist("new " + clazz, "method " + clazz + " getProp5", "method " + clazz + " setProp5 java.lang.String", "method " + clazz + " rawProp5"), "EDITEDedited", "def c = new " + clazz + "(); c.prop5 = 'EDITED'; c.prop5 + c.rawProp5()");
assertRejected(new StaticWhitelist("new " + clazz), "field " + clazz + " prop5", "new " + clazz + "().@prop5");
assertEvaluate(new StaticWhitelist("new " + clazz, "field " + clazz + " prop5"), "default", "new " + clazz + "().@prop5");
assertRejected(new StaticWhitelist("new " + clazz, "method " + clazz + " getProp5"), "field " + clazz + " prop5", "def c = new " + clazz + "(); c.@prop5 = 'edited'; c.prop5");
assertEvaluate(new StaticWhitelist("new " + clazz, "method " + clazz + " getProp5", "field " + clazz + " prop5"), "EDITED", "def c = new " + clazz + "(); c.@prop5 = 'edited'; c.prop5");
}

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

0 comments on commit e7d3bc9

Please sign in to comment.