CVE-2020-2109

@@ -70,7 +70,7 @@
         <git-plugin.version>3.1.0</git-plugin.version>
         <workflow-support-plugin.version>3.3</workflow-support-plugin.version>
         <scm-api-plugin.version>2.2.6</scm-api-plugin.version>
-        <groovy-cps.version>1.31</groovy-cps.version>
+        <groovy-cps.version>1.32</groovy-cps.version>
         <structs-plugin.version>1.20</structs-plugin.version>
         <workflow-step-api-plugin.version>2.21</workflow-step-api-plugin.version>
     </properties>

@@ -759,4 +759,12 @@ public void scriptInitializerCallsCpsTransformedMethod() throws Exception {
         jenkins.assertLogContains("staticMethod jenkins.model.Jenkins getInstance", b4);
     }
 
+    @Issue("SECURITY-1710")
+    @Test public void blockInitialExpressionsForParamsInCpsTransformedMethods() throws Exception {
+        WorkflowJob p = jenkins.createProject(WorkflowJob.class);
+        p.setDefinition(new CpsFlowDefinition("def m(p = Jenkins.getInstance()) { true }; m()", true));
+        WorkflowRun b = jenkins.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0));
+        jenkins.assertLogContains("staticMethod jenkins.model.Jenkins getInstance", b);
+    }
+
 }

@@ -4,7 +4,7 @@
     <parent>
         <groupId>com.cloudbees</groupId>
         <artifactId>groovy-cps-parent</artifactId>
-        <version>1.32-SNAPSHOT</version>
+        <version>${revision}${changelist}</version>
     </parent>
 
     <artifactId>groovy-cps-dgm-builder</artifactId>

@@ -4,7 +4,7 @@
   <parent>
     <groupId>com.cloudbees</groupId>
     <artifactId>groovy-cps-parent</artifactId>
-    <version>1.32-SNAPSHOT</version>
+    <version>${revision}${changelist}</version>
   </parent>
 
   <artifactId>groovy-cps</artifactId>

@@ -123,6 +123,10 @@ public void call(SourceUnit source, GeneratorContext context, ClassNode classNod
         }
         this.sourceUnit = source;
         this.classNode = classNode;
+
+        // Removes all initial expressions for methods and constructors and generates overloads for all variants.
+        new InitialExpressionExpander().expandInitialExpressions(classNode);
+
         try {
 
             for (FieldNode field : new ArrayList<>(classNode.getFields())) {
@@ -192,17 +196,22 @@ boolean hasAnnotation(MethodNode node, Class<? extends Annotation> a) {
      *
      * From:
      *
+     * <pre>{@code
      * ReturnT foo( T1 arg1, T2 arg2, ...) { ... body ... }
+     * }</pre>
      *
      * To:
      *
+     * <pre>{@code
      * private static CpsFunction ___cps___N = ___cps___N();
      *
-     * private static final CpsFunction ___cps___N() { return new
-     * CpsFunction(['arg1','arg2','arg3',...], CPS-transformed-method-body) }
+     * private static final CpsFunction ___cps___N() {
+     *     return new CpsFunction(['arg1','arg2','arg3',...], CPS-transformed-method-body)
+     * }
      *
-     * ReturnT foo( T1 arg1, T2 arg2, ...) { throw new
-     * CpsCallableInvocation(___cps___N, this, new Object[] {arg1, arg2, ...}) }
+     * ReturnT foo( T1 arg1, T2 arg2, ...) {
+     *     throw new CpsCallableInvocation(___cps___N, this, new Object[] {arg1, arg2, ...}) }
+     * }</pre>
      */
     public void visitMethod(final MethodNode m) {
         if (!shouldBeTransformed(m)) {
@@ -907,6 +916,10 @@ public void run() {
                     Parameter[] paramArray = exp.getParameters();
                     List<Expression> typesList = new ArrayList<Expression>(paramArray.length);
                     List<Expression> paramsList = new ArrayList<Expression>(paramArray.length);
+                    // Note: This code currently ignores initial expressions for closure parameters.
+                    // Be careful that any refactoring either maintains the status quo, or transforms these
+                    // initial expressions in such a way that they are correctly intercepted by the sandbox.
+                    // See SandboxInvoker2Test.closureParametersWithInitialExpressions, which is currently ignored.
                     for (Parameter p : paramArray) {
                         typesList.add(new ClassExpression(p.getType()));
                         paramsList.add(new ConstantExpression(p.getName()));
@@ -1297,6 +1310,77 @@ public void visitBytecodeExpression(BytecodeExpression expression) {
                 expression.getLineNumber(), expression.getColumnNumber()));
     }
 
+    // Required because the methods we need have protected visibility in Verifier.
+    private static class InitialExpressionExpander extends Verifier {
+        private void expandInitialExpressions(ClassNode node) {
+            super.setClassNode(node);
+            super.addDefaultParameterMethods(node);
+            fixupVariableReferencesInGeneratedMethods(node);
+        }
+        // Methods generated by Verifier.addDefaultParameterMethods have some VariableExpressions where
+        // VariableExpression.getAccessedVariable returns null, when it should return the same variable. This causes
+        // problems in visitVariableExpression, so this method prevents errors by setting the accessed variable to the
+        // same variable here. The variables we need to fix are always in a cast expression in a method call expression
+        // in the final statement of the body of the generated method.
+        private void fixupVariableReferencesInGeneratedMethods(ClassNode node) {
+            for (MethodNode m : node.getMethods()) {
+                if (Boolean.TRUE.equals(m.getNodeMetaData(Verifier.DEFAULT_PARAMETER_GENERATED))) {
+                    MethodCallExpression mce = findMethodCallExpressionInLastStatementOfBody(m);
+                    if (mce != null) {
+                        for (Expression arg : ((TupleExpression)mce.getArguments()).getExpressions()) {
+                            if (arg instanceof CastExpression) {
+                                Expression castedExpr = ((CastExpression) arg).getExpression();
+                                if (castedExpr instanceof VariableExpression) {
+                                    VariableExpression varExpr = (VariableExpression) castedExpr;
+                                    if (varExpr.getAccessedVariable() == null) {
+                                        varExpr.setAccessedVariable(varExpr);
+                                    } else {
+                                        LOGGER.log(Level.FINE, "Variable {0} in method call arguments already had an initial expression", varExpr);
+                                    }
+                                } else {
+                                    // The initial expressions are inlined directly into the arguments in the
+                                    // MethodCallExpression, so if this isn't a variable expression, we just ignore it.
+                                }
+                            } else {
+                                LOGGER.log(Level.FINE, "Unexpected expression in method call arguments in {0}: {1}", new Object[]{ m, arg });
+                            }
+                        }
+                    }
+                }
+            }
+        }
+        private MethodCallExpression findMethodCallExpressionInLastStatementOfBody(MethodNode m) {
+            if (m.getCode() instanceof BlockStatement) {
+                List<Statement> body = ((BlockStatement)m.getCode()).getStatements();
+                if (!body.isEmpty()) {
+                    Statement finalStatement = body.get(body.size() - 1);
+                    if (finalStatement instanceof ReturnStatement) { // For methods with return values.
+                        Expression returnExpr = ((ReturnStatement) finalStatement).getExpression();
+                        if (returnExpr instanceof MethodCallExpression) {
+                            return (MethodCallExpression) returnExpr;
+                        } else {
+                            LOGGER.log(Level.FINE, "Unexpected expression in return statement of {0}: {1}", new Object[]{ m, returnExpr });
+                        }
+                    } else if (finalStatement instanceof ExpressionStatement) { // For void methods.
+                        Expression expr = ((ExpressionStatement) finalStatement).getExpression();
+                        if (expr instanceof MethodCallExpression) {
+                            return (MethodCallExpression) expr;
+                        } else {
+                            LOGGER.log(Level.FINE, "Unexpected expression in last statement of {0}: {1}", new Object[]{ m, expr });
+                        }
+                    } else {
+                        LOGGER.log(Level.FINE, "Unexpected type of last statement of {0}: {1}", new Object[]{ m, finalStatement });
+                    }
+                } else {
+                    LOGGER.log(Level.FINE, "Body of {0} is empty", m);
+                }
+            } else {
+                LOGGER.log(Level.FINE, "Body of {0} is not a block statement", m);
+            }
+            return null;
+        }
+    }
+
     private static final ClassNode OBJECT_TYPE = ClassHelper.makeCached(Object.class);
 
     private static final ClassNode FUNCTION_TYPE = ClassHelper.makeCached(CpsFunction.class);

@@ -0,0 +1,77 @@
+/*
+ * Copyright 2020 CloudBees, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.cloudbees.groovy.cps;
+
+import java.util.Arrays;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class CpsTransformer2Test extends AbstractGroovyCpsTest {
+
+    @Test
+    public void initialExpressionsInMethodsAreCpsTransformed() throws Throwable {
+        assertEquals(Boolean.FALSE, evalCPS(
+                "def m1() { true }\n" +
+                "def m2(p = m1()){ false }\n" +
+                "m2()\n"));
+    }
+
+    @Test public void methodsWithInitialExpressionsAreExpandedToCorrectOverloads() throws Throwable {
+        assertEquals(Arrays.asList("abc", "xbc", "xyc", "xyz"), evalCPS(
+                "def m2(a = 'a', b = 'b', c = 'c') {\n" +
+                "    a + b + c\n" +
+                "}\n" +
+                "def r1 = m2()\n" +
+                "def r2 = m2('x')\n" +
+                "def r3 = m2('x', 'y')\n" +
+                "def r4 = m2('x', 'y', 'z')\n" +
+                "[r1, r2, r3, r4]"));
+        assertEquals(Arrays.asList("abc", "xbc", "xby"), evalCPS(
+                "def m2(a = 'a', b, c = 'c') {\n" +
+                "    a + b + c\n" +
+                "}\n" +
+                "def r1 = m2('b')\n" +
+                "def r2 = m2('x', 'b')\n" +
+                "def r3 = m2('x', 'b', 'y')\n" +
+                "[r1, r2, r3]"));
+    }
+
+    @Test public void voidMethodsWithInitialExpressionsAreExpandedToCorrectOverloads() throws Throwable {
+        assertEquals(Arrays.asList("abc", "xbc", "xyc", "xyz"), evalCPS(
+                "import groovy.transform.Field\n" +
+                "@Field def r = []\n" +
+                "void m2(a = 'a', b = 'b', c = 'c') {\n" +
+                "    r.add(a + b + c)\n" +
+                "}\n" +
+                "m2()\n" +
+                "m2('x')\n" +
+                "m2('x', 'y')\n" +
+                "m2('x', 'y', 'z')\n" +
+                "r"));
+        assertEquals(Arrays.asList("abc", "xbc", "xby"), evalCPS(
+                "import groovy.transform.Field\n" +
+                "@Field def r = []\n" +
+                "void m2(a = 'a', b, c = 'c') {\n" +
+                "    r.add(a + b + c)\n" +
+                "}\n" +
+                "m2('b')\n" +
+                "m2('x', 'b')\n" +
+                "m2('x', 'b', 'y')\n" +
+                "r"));
+    }
+}

@@ -0,0 +1,95 @@
+/*
+ * Copyright 2020 CloudBees, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.cloudbees.groovy.cps.sandbox;
+
+import com.cloudbees.groovy.cps.Continuation;
+import com.cloudbees.groovy.cps.CpsTransformer;
+import com.cloudbees.groovy.cps.Envs;
+import com.cloudbees.groovy.cps.SandboxCpsTransformer;
+import com.cloudbees.groovy.cps.AbstractGroovyCpsTest;
+import com.cloudbees.groovy.cps.impl.FunctionCallEnv;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.kohsuke.groovy.sandbox.ClassRecorder;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.junit.Assert.assertThat;
+
+public class SandboxInvoker2Test extends AbstractGroovyCpsTest {
+    ClassRecorder cr = new ClassRecorder();
+
+    @Override
+    protected CpsTransformer createCpsTransformer() {
+        return new SandboxCpsTransformer();
+    }
+
+    @Before public void zeroIota() {
+        CpsTransformer.iota.set(0);
+    }
+
+    private Object evalCpsSandbox(String script) throws Throwable {
+        FunctionCallEnv e = (FunctionCallEnv)Envs.empty();
+        e.setInvoker(new SandboxInvoker());
+
+        cr.register();
+        try {
+            return parseCps(script).invoke(e, null, Continuation.HALT).run().yield.replay();
+        } finally {
+            cr.unregister();
+        }
+    }
+
+    public void assertIntercept(String... expected) {
+        assertThat(cr.toString().split("\n"), equalTo(expected));
+    }
+
+    @Issue("SECURITY-1710")
+    @Test public void methodParametersWithInitialExpressions() throws Throwable {
+        evalCpsSandbox("def m(p = System.getProperties()){ true }; m()");
+        assertIntercept(
+                "Script1.super(Script1).setBinding(Binding)",
+                "Script1.m()",
+                "System:getProperties()",
+                "Checker:checkedCast(Class,Properties,Boolean,Boolean,Boolean)",
+                "Script1.m(Properties)");
+    }
+
+    @Test public void constructorParametersWithInitialExpressions() throws Throwable {
+        evalCpsSandbox(
+                "class Test {\n" +
+                "  Test(p = System.getProperties()) { }" +
+                "}\n" +
+                "new Test()");
+        assertIntercept(
+                "Script1.super(Script1).setBinding(Binding)",
+                "new Test()",
+                "System:getProperties()");
+    }
+
+    @Ignore("Initial expressions for parameters in CPS-transformed closures are currently ignored")
+    @Test public void closureParametersWithInitialExpressions() throws Throwable {
+        // Fails because p is null in the body of the closure.
+        assertEquals(true, evalCpsSandbox("{ p = System.getProperties() -> p != null }()"));
+        assertIntercept(
+                "Script1.super(Script1).setBinding(Binding)",
+                "CpsClosure.call()",
+                "System:getProperties()", // Not currently intercepted because it is dropped by the transformer.
+                "ScriptBytecodeAdapter:compareNotEqual(null,null)");
+    }
+}

@@ -0,0 +1,7 @@
+<extensions xmlns="http://maven.apache.org/EXTENSIONS/1.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/EXTENSIONS/1.0.0 http://maven.apache.org/xsd/core-extensions-1.0.0.xsd">
+  <extension>
+    <groupId>io.jenkins.tools.incrementals</groupId>
+    <artifactId>git-changelist-maven-extension</artifactId>
+    <version>1.1</version>
+  </extension>
+</extensions>

@@ -0,0 +1,2 @@
+-Pconsume-incrementals
+-Pmight-produce-incrementals

@@ -9,20 +9,27 @@
     </parent>
 
     <artifactId>groovy-cps-parent</artifactId>
-    <version>1.32-SNAPSHOT</version>
+    <version>${revision}${changelist}</version>
     <packaging>pom</packaging>
 
     <name>Groovy CPS Execution Parent</name>
 
     <properties>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <groovy.version>2.4.7</groovy.version>
+        <revision>1.32</revision>
+        <changelist>-SNAPSHOT</changelist>
+        <!-- TODO: Move these three properties to the parent POM or use org.jenkins-ci:jenkins as the parent POM here -->
+        <incrementals-enforce-minimum.version>1.1</incrementals-enforce-minimum.version>
+        <incrementals-plugin.version>1.1</incrementals-plugin.version>
+        <incrementals.url>https://repo.jenkins-ci.org/incrementals/</incrementals.url>
+        <scmTag>HEAD</scmTag>
     </properties>
 
     <scm>
         <connection>scm:git:git@github.com/cloudbees/groovy-cps.git</connection>
         <developerConnection>scm:git:ssh://git@github.com/cloudbees/groovy-cps.git</developerConnection>
-        <tag>HEAD</tag>
+        <tag>${scmTag}</tag>
     </scm>
 
     <licenses>
@@ -38,4 +45,153 @@
         <module>lib</module>
     </modules>
 
+    <build>
+        <pluginManagement>
+            <plugins>
+                <plugin>
+                    <groupId>io.jenkins.tools.incrementals</groupId>
+                    <artifactId>incrementals-maven-plugin</artifactId>
+                    <version>${incrementals-plugin.version}</version>
+                    <configuration>
+                        <includes>
+                            <include>org.jenkins-ci.*</include>
+                            <include>io.jenkins.*</include>
+                        </includes>
+                        <generateBackupPoms>false</generateBackupPoms>
+                        <updateNonincremental>false</updateNonincremental>
+                    </configuration>
+                </plugin>
+            </plugins>
+        </pluginManagement>
+    </build>
+
+    <!-- TODO: Move these profiles to the parent POM or use org.jenkins-ci:jenkins as the parent POM here -->
+    <profiles>
+        <profile> <!-- see JEP-305 -->
+            <id>consume-incrementals</id>
+            <repositories>
+                <repository>
+                    <id>incrementals</id>
+                    <url>${incrementals.url}</url>
+                    <snapshots>
+                        <enabled>false</enabled>
+                    </snapshots>
+                </repository>
+            </repositories>
+            <pluginRepositories>
+                <pluginRepository>
+                    <id>incrementals</id>
+                    <url>${incrementals.url}</url>
+                    <snapshots>
+                        <enabled>false</enabled>
+                    </snapshots>
+                </pluginRepository>
+            </pluginRepositories>
+        </profile>
+        <profile>
+            <id>might-produce-incrementals</id>
+            <build>
+                <plugins>
+                    <plugin>
+                        <groupId>org.codehaus.mojo</groupId>
+                        <artifactId>flatten-maven-plugin</artifactId>
+                        <version>1.0.1</version>
+                        <configuration>
+                            <updatePomFile>true</updatePomFile>
+                            <outputDirectory>${project.build.directory}</outputDirectory>
+                            <flattenedPomFilename>${project.artifactId}-${project.version}.pom</flattenedPomFilename>
+                        </configuration>
+                        <executions>
+                            <execution>
+                                <id>flatten</id>
+                                <phase>process-resources</phase>
+                                <goals>
+                                    <goal>flatten</goal>
+                                </goals>
+                                <configuration>
+                                    <flattenMode>oss</flattenMode>
+                                </configuration>
+                            </execution>
+                        </executions>
+                    </plugin>
+                    <plugin>
+                        <artifactId>maven-enforcer-plugin</artifactId>
+                        <version>3.0.0-M2</version>
+                        <executions>
+                            <execution>
+                                <id>display-info</id>
+                                <configuration>
+                                    <rules>
+                                        <requireMavenVersion>
+                                            <version>[3.5.4,)</version>
+                                            <message>3.5.4+ required to use Incrementals.</message>
+                                        </requireMavenVersion>
+                                        <rule implementation="io.jenkins.tools.incrementals.enforcer.RequireExtensionVersion">
+                                            <version>[${incrementals-enforce-minimum.version},)</version>
+                                        </rule>
+                                    </rules>
+                                </configuration>
+                            </execution>
+                        </executions>
+                        <dependencies>
+                            <dependency>
+                                <groupId>io.jenkins.tools.incrementals</groupId>
+                                <artifactId>incrementals-enforcer-rules</artifactId>
+                                <version>${incrementals-plugin.version}</version>
+                            </dependency>
+                        </dependencies>
+                    </plugin>
+                    <plugin>
+                        <artifactId>maven-release-plugin</artifactId>
+                        <configuration>
+                            <completionGoals>incrementals:reincrementalify</completionGoals>
+                        </configuration>
+                    </plugin>
+                </plugins>
+            </build>
+        </profile>
+        <profile>
+            <id>produce-incrementals</id>
+            <activation>
+                <property>
+                    <name>set.changelist</name>
+                    <value>true</value>
+                </property>
+            </activation>
+            <distributionManagement>
+                <repository>
+                    <id>incrementals</id>
+                    <url>${incrementals.url}</url>
+                </repository>
+            </distributionManagement>
+            <build>
+                <plugins>
+                    <plugin>
+                        <artifactId>maven-source-plugin</artifactId>
+                        <version>3.0.1</version>
+                        <executions>
+                            <execution>
+                                <id>attach-sources</id>
+                                <goals>
+                                    <goal>jar-no-fork</goal>
+                                </goals>
+                            </execution>
+                        </executions>
+                    </plugin>
+                    <plugin>
+                        <artifactId>maven-javadoc-plugin</artifactId>
+                        <executions>
+                            <execution>
+                                <id>attach-javadocs</id>
+                                <goals>
+                                    <goal>jar</goal>
+                                </goals>
+                            </execution>
+                        </executions>
+                    </plugin>
+                </plugins>
+            </build>
+        </profile>
+    </profiles>
+
 </project>