Skip to content

Commit 2c5122e

Browse files
committed
[SECURITY-1266] Block problematic AST transforms from sandbox
1 parent a559061 commit 2c5122e

File tree

3 files changed

+124
-1
lines changed

3 files changed

+124
-1
lines changed

src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,12 @@
2525
package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy;
2626

2727
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
28+
import groovy.grape.GrabAnnotationTransformation;
2829
import groovy.lang.GroovyShell;
2930
import groovy.lang.Script;
3031

32+
import java.util.Collections;
33+
import java.util.HashSet;
3134
import java.util.concurrent.Callable;
3235
import javax.annotation.Nonnull;
3336
import org.codehaus.groovy.control.CompilerConfiguration;
@@ -58,11 +61,21 @@ public class GroovySandbox {
5861
* @return a compiler configuration set up to use the sandbox
5962
*/
6063
public static @Nonnull CompilerConfiguration createSecureCompilerConfiguration() {
61-
CompilerConfiguration cc = new CompilerConfiguration();
64+
CompilerConfiguration cc = createBaseCompilerConfiguration();
6265
cc.addCompilationCustomizers(new SandboxTransformer());
6366
return cc;
6467
}
6568

69+
/**
70+
* Prepares a compiler configuration that rejects certain AST transformations. Used by {@link #createSecureCompilerConfiguration()}.
71+
*/
72+
public static @Nonnull CompilerConfiguration createBaseCompilerConfiguration() {
73+
CompilerConfiguration cc = new CompilerConfiguration();
74+
cc.addCompilationCustomizers(new RejectASTTransformsCustomizer());
75+
cc.setDisabledGlobalASTTransformations(new HashSet<>(Collections.singletonList(GrabAnnotationTransformation.class.getName())));
76+
return cc;
77+
}
78+
6679
/**
6780
* Prepares a classloader for Groovy shell for sandboxing.
6881
*
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* The MIT License
3+
*
4+
* Copyright (c) 2018, CloudBees, Inc.
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy
7+
* of this software and associated documentation files (the "Software"), to deal
8+
* in the Software without restriction, including without limitation the rights
9+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
* copies of the Software, and to permit persons to whom the Software is
11+
* furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in
14+
* all copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
* THE SOFTWARE.
23+
*/
24+
25+
package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy;
26+
27+
import com.google.common.collect.ImmutableList;
28+
import groovy.lang.Grab;
29+
import groovy.transform.ASTTest;
30+
import org.codehaus.groovy.ast.AnnotatedNode;
31+
import org.codehaus.groovy.ast.AnnotationNode;
32+
import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
33+
import org.codehaus.groovy.ast.ClassNode;
34+
import org.codehaus.groovy.classgen.GeneratorContext;
35+
import org.codehaus.groovy.control.CompilationFailedException;
36+
import org.codehaus.groovy.control.CompilePhase;
37+
import org.codehaus.groovy.control.SourceUnit;
38+
import org.codehaus.groovy.control.customizers.CompilationCustomizer;
39+
40+
import java.lang.annotation.Annotation;
41+
import java.util.List;
42+
43+
public class RejectASTTransformsCustomizer extends CompilationCustomizer {
44+
private static final List<Class<? extends Annotation>> BLOCKED_TRANSFORMS = ImmutableList.of(ASTTest.class, Grab.class);
45+
46+
public RejectASTTransformsCustomizer() {
47+
super(CompilePhase.CONVERSION);
48+
}
49+
50+
@Override
51+
public void call(final SourceUnit source, GeneratorContext context, ClassNode classNode) throws CompilationFailedException {
52+
new RejectASTTransformsVisitor(source).visitClass(classNode);
53+
}
54+
55+
private static class RejectASTTransformsVisitor extends ClassCodeVisitorSupport {
56+
private SourceUnit source;
57+
58+
public RejectASTTransformsVisitor(SourceUnit source) {
59+
this.source = source;
60+
}
61+
62+
@Override
63+
protected SourceUnit getSourceUnit() {
64+
return source;
65+
}
66+
67+
/**
68+
* If the node is annotated with one of the blocked transform annotations, throw a security exception.
69+
*
70+
* @param node the node to process
71+
*/
72+
@Override
73+
public void visitAnnotations(AnnotatedNode node) {
74+
for (AnnotationNode an : node.getAnnotations()) {
75+
for (Class<? extends Annotation> blockedAnnotation : BLOCKED_TRANSFORMS) {
76+
if (blockedAnnotation.getSimpleName().equals(an.getClassNode().getName())) {
77+
throw new SecurityException("Annotation " + blockedAnnotation.getSimpleName() + " cannot be used in the sandbox.");
78+
}
79+
}
80+
}
81+
}
82+
}
83+
}

src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,15 @@
7474
import org.junit.Rule;
7575
import org.junit.Test;
7676
import org.junit.rules.ErrorCollector;
77+
import org.junit.rules.ExpectedException;
7778
import org.jvnet.hudson.test.Issue;
7879

7980
public class SandboxInterceptorTest {
8081

8182
@Rule public ErrorCollector errors = new ErrorCollector();
8283

84+
@Rule public ExpectedException thrown = ExpectedException.none();
85+
8386
@Test public void genericWhitelist() throws Exception {
8487
assertEvaluate(new GenericWhitelist(), 3, "'foo bar baz'.split(' ').length");
8588
assertEvaluate(new GenericWhitelist(), false, "def x = null; x != null");
@@ -818,6 +821,30 @@ public void explode() {}
818821
assertRejected(new StaticWhitelist(), "staticMethod hudson.model.Hudson getInstance", "hudson.model.Hudson.instance");
819822
}
820823

824+
@Issue("SECURITY-1266")
825+
@Test
826+
public void blockedASTTransformsASTTest() throws Exception {
827+
GroovyShell shell = new GroovyShell(GroovySandbox.createSecureCompilerConfiguration());
828+
829+
thrown.expect(MultipleCompilationErrorsException.class);
830+
thrown.expectMessage("Annotation ASTTest cannot be used in the sandbox");
831+
832+
shell.parse("import groovy.transform.*\n" +
833+
"@ASTTest(value={ assert true })\n" +
834+
"@Field int x\n");
835+
}
836+
837+
@Issue("SECURITY-1266")
838+
@Test
839+
public void blockedASTTransformsGrab() throws Exception {
840+
GroovyShell shell = new GroovyShell(GroovySandbox.createSecureCompilerConfiguration());
841+
thrown.expect(MultipleCompilationErrorsException.class);
842+
thrown.expectMessage("Annotation Grab cannot be used in the sandbox");
843+
844+
shell.parse("@Grab(group='foo', module='bar', version='1.0')\n" +
845+
"def foo\n");
846+
}
847+
821848
private static Object evaluate(Whitelist whitelist, String script) {
822849
GroovyShell shell = new GroovyShell(GroovySandbox.createSecureCompilerConfiguration());
823850
Object actual = GroovySandbox.run(shell.parse(script), whitelist);

0 commit comments

Comments
 (0)