Skip to content

Commit

Permalink
[FIXED SECURITY-1318, SECURITY-1319, SECURITY-1320, SECURITY-1321]
Browse files Browse the repository at this point in the history 
Block all Grab-related annotations, block fully qualified class names
for blocked annotations, block AnnotationCollector, block imports of
blocked annotations as well to avoid aliasing issues.
  • Loading branch information
@abayer
abayer committed Feb 14, 2019
1 parent ffc50e2 commit 3228c88
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 5 deletions.
56 changes: 51 additions & 5 deletions ...va/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/RejectASTTransformsCustomizer.java
View file
Expand Up @@ -26,22 +26,32 @@

import com.google.common.collect.ImmutableList;
import groovy.lang.Grab;
import groovy.lang.GrabConfig;
import groovy.lang.GrabExclude;
import groovy.lang.GrabResolver;
import groovy.lang.Grapes;
import groovy.transform.ASTTest;
import groovy.transform.AnnotationCollector;
import org.codehaus.groovy.ast.AnnotatedNode;
import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.ImportNode;
import org.codehaus.groovy.ast.ModuleNode;
import org.codehaus.groovy.classgen.GeneratorContext;
import org.codehaus.groovy.control.CompilationFailedException;
import org.codehaus.groovy.control.CompilePhase;
import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.control.customizers.CompilationCustomizer;

import java.lang.annotation.Annotation;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class RejectASTTransformsCustomizer extends CompilationCustomizer {
private static final List<Class<? extends Annotation>> BLOCKED_TRANSFORMS = ImmutableList.of(ASTTest.class, Grab.class);
private static final List<String> BLOCKED_TRANSFORMS = ImmutableList.of(ASTTest.class.getCanonicalName(), Grab.class.getCanonicalName(),
GrabConfig.class.getCanonicalName(), GrabExclude.class.getCanonicalName(), GrabResolver.class.getCanonicalName(),
Grapes.class.getCanonicalName(), AnnotationCollector.class.getCanonicalName());

public RejectASTTransformsCustomizer() {
super(CompilePhase.CONVERSION);
Expand All @@ -64,6 +74,28 @@ protected SourceUnit getSourceUnit() {
return source;
}

@Override
public void visitImports(ModuleNode node) {
if (node != null) {
for (ImportNode importNode : node.getImports()) {
checkImportForBlockedAnnotation(importNode);
}
for (ImportNode importStaticNode : node.getStaticImports().values()) {
checkImportForBlockedAnnotation(importStaticNode);
}
}
}

private void checkImportForBlockedAnnotation(ImportNode node) {
if (node != null && node.getType() != null) {
for (String blockedAnnotation : getBlockedTransforms()) {
if (blockedAnnotation.equals(node.getType().getName()) || blockedAnnotation.endsWith("." + node.getType().getName())) {
throw new SecurityException("Annotation " + node.getType().getName() + " cannot be used in the sandbox.");
}
}
}
}

/**
* If the node is annotated with one of the blocked transform annotations, throw a security exception.
*
Expand All @@ -72,12 +104,26 @@ protected SourceUnit getSourceUnit() {
@Override
public void visitAnnotations(AnnotatedNode node) {
for (AnnotationNode an : node.getAnnotations()) {
for (Class<? extends Annotation> blockedAnnotation : BLOCKED_TRANSFORMS) {
if (blockedAnnotation.getSimpleName().equals(an.getClassNode().getName())) {
throw new SecurityException("Annotation " + blockedAnnotation.getSimpleName() + " cannot be used in the sandbox.");
for (String blockedAnnotation : getBlockedTransforms()) {
if (blockedAnnotation.equals(an.getClassNode().getName()) || blockedAnnotation.endsWith("." + an.getClassNode().getName())) {
throw new SecurityException("Annotation " + an.getClassNode().getName() + " cannot be used in the sandbox.");
}
}
}
}
}

private static List<String> getBlockedTransforms() {
List<String> blocked = new ArrayList<>(BLOCKED_TRANSFORMS);

String additionalBlocked = System.getProperty(RejectASTTransformsCustomizer.class.getName() + ".ADDITIONAL_BLOCKED_TRANSFORMS");

if (additionalBlocked != null) {
for (String b : additionalBlocked.split(",")) {
blocked.add(b.trim());
}
}

return blocked;
}
}
86 changes: 86 additions & 0 deletions ...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java
View file
Expand Up @@ -873,5 +873,91 @@ public void blockGrab() throws Exception {
containsString("Annotation Grab cannot be used in the sandbox"));
}

@Issue("SECURITY-1318")
@Test
public void blockGrapes() throws Exception {
SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class);
assertThat(d.doCheckScript("@Grapes([@Grab(group='foo', module='bar', version='1.0')])\ndef foo\n", false).toString(),
containsString("Annotation Grapes cannot be used in the sandbox"));
}

@Issue("SECURITY-1318")
@Test
public void blockGrabConfig() throws Exception {
SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class);
assertThat(d.doCheckScript("@GrabConfig(autoDownload=false)\ndef foo\n", false).toString(),
containsString("Annotation GrabConfig cannot be used in the sandbox"));
}

@Issue("SECURITY-1318")
@Test
public void blockGrabExclude() throws Exception {
SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class);
assertThat(d.doCheckScript("@GrabExclude(group='org.mortbay.jetty', module='jetty-util')\ndef foo\n", false).toString(),
containsString("Annotation GrabExclude cannot be used in the sandbox"));
}

@Issue("SECURITY-1319")
@Test
public void blockGrabResolver() throws Exception {
SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class);
assertThat(d.doCheckScript("@GrabResolver(name='restlet.org', root='http://maven.restlet.org')\ndef foo\n", false).toString(),
containsString("Annotation GrabResolver cannot be used in the sandbox"));
}

@Issue("SECURITY-1318")
@Test
public void blockArbitraryAnnotation() throws Exception {
try {
System.setProperty(RejectASTTransformsCustomizer.class.getName() + ".ADDITIONAL_BLOCKED_TRANSFORMS", "groovy.transform.Field,groovy.transform.Immutable");
SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class);
assertThat(d.doCheckScript("@Field\ndef foo\n", false).toString(),
containsString("Annotation Field cannot be used in the sandbox"));
} finally {
System.clearProperty(RejectASTTransformsCustomizer.class.getName() + ".ADDITIONAL_BLOCKED_TRANSFORMS");
}
}

@Issue("SECURITY-1321")
@Test
public void blockAnnotationCollector() throws Exception {
SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class);
assertThat(d.doCheckScript("import groovy.transform.*\n" +
"import jenkins.model.Jenkins\n" +
"import hudson.model.FreeStyleProject\n" +
"@AnnotationCollector([ASTTest]) @interface Lol {}\n" +
"@Lol(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, \"should-not-exist\") })\n" +
"@Field int x\n" +
"echo 'hello'\n", false).toString(), containsString("Annotation AnnotationCollector cannot be used in the sandbox"));

assertNull(r.jenkins.getItem("should-not-exist"));
}

@Issue("SECURITY-1320")
@Test
public void blockFQCN() throws Exception {
SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class);
assertThat(d.doCheckScript("import groovy.transform.*\n" +
"import jenkins.model.Jenkins\n" +
"import hudson.model.FreeStyleProject\n" +
"@groovy.transform.ASTTest(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, \"should-not-exist\") })\n" +
"@Field int x\n" +
"echo 'hello'\n", false).toString(), containsString("Annotation groovy.transform.ASTTest cannot be used in the sandbox"));

assertNull(r.jenkins.getItem("should-not-exist"));
}

@Issue("SECURITY-1320")
@Test
public void blockImportAsBlockedAnnotation() throws Exception {
SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class);
assertThat(d.doCheckScript("import groovy.transform.ASTTest as lolwut\n" +
"import jenkins.model.Jenkins\n" +
"import hudson.model.FreeStyleProject\n" +
"@lolwut(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, \"should-not-exist\") })\n" +
"int x\n" +
"echo 'hello'\n", false).toString(), containsString("Annotation groovy.transform.ASTTest cannot be used in the sandbox"));

assertNull(r.jenkins.getItem("should-not-exist"));
}
}

0 comments on commit 3228c88

Please sign in to comment.