CVE-2022-25173

@@ -260,6 +260,17 @@
             <artifactId>pipeline-stage-step</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins</groupId>
+            <artifactId>subversion</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins</groupId>
+            <artifactId>subversion</artifactId>
+            <classifier>tests</classifier>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.testcontainers</groupId>
             <artifactId>testcontainers</artifactId>
@@ -277,6 +288,12 @@
                 </exclusion>
             </exclusions>
         </dependency>
+        <dependency>
+            <groupId>org.tmatesoft.svnkit</groupId>
+            <artifactId>svnkit-cli</artifactId>
+            <version>1.10.1</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
     <build>
         <resources>

@@ -40,6 +40,7 @@
 import hudson.scm.SCM;
 import hudson.scm.SCMDescriptor;
 import hudson.slaves.WorkspaceList;
+import java.io.File;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -48,6 +49,7 @@
 import java.util.List;
 import jenkins.model.Jenkins;
 import jenkins.scm.api.SCMFileSystem;
+import jenkins.security.HMACConfidentialKey;
 import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
 import static org.jenkinsci.plugins.workflow.cps.persistence.PersistenceContext.JOB;
 
@@ -68,6 +70,8 @@
 @PersistIn(JOB)
 public class CpsScmFlowDefinition extends FlowDefinition {
 
+    private static final HMACConfidentialKey CHECKOUT_DIR_KEY = new HMACConfidentialKey(CpsScmFlowDefinition.class, "filePathWithSuffix", 32);
+
     private final SCM scm;
     private final String scriptPath;
     private boolean lightweight;
@@ -134,7 +138,7 @@ public boolean isLightweight() {
             if (baseWorkspace == null) {
                 throw new IOException(node.getDisplayName() + " may be offline");
             }
-            dir = getFilePathWithSuffix(baseWorkspace);
+            dir = getFilePathWithSuffix(baseWorkspace, scm);
         } else { // should not happen, but just in case:
             dir = new FilePath(owner.getRootDir());
         }
@@ -149,6 +153,7 @@ public boolean isLightweight() {
         delegate.setChangelog(true);
         FilePath acquiredDir;
         try (WorkspaceList.Lease lease = computer.getWorkspaceList().acquire(dir)) {
+            dir.withSuffix("-scm-key.txt").write(scm.getKey(), "UTF-8");
             for (int retryCount = Jenkins.get().getScmCheckoutRetryCount(); retryCount >= 0; retryCount--) {
                 try {
                     delegate.checkout(build, dir, listener, node.createLauncher(listener));
@@ -174,8 +179,8 @@ public boolean isLightweight() {
             }
 
             FilePath scriptFile = dir.child(expandedScriptPath);
-            if (!scriptFile.absolutize().getRemote().replace('\\', '/').startsWith(dir.absolutize().getRemote().replace('\\', '/') + '/')) { // TODO JENKINS-26838
-                throw new IOException(scriptFile + " is not inside " + dir);
+            if (!new File(scriptFile.getRemote()).getCanonicalFile().toPath().startsWith(dir.absolutize().getRemote())) { // TODO JENKINS-26838
+                throw new IOException(scriptFile + " references a file that is not inside " + dir);
             }
             if (!scriptFile.exists()) {
                 throw new AbortException(scriptFile + " not found");
@@ -190,8 +195,8 @@ public boolean isLightweight() {
         return exec;
     }
 
-    private FilePath getFilePathWithSuffix(FilePath baseWorkspace) {
-        return baseWorkspace.withSuffix(getFilePathSuffix() + "script");
+    private FilePath getFilePathWithSuffix(FilePath baseWorkspace, SCM scm) {
+        return baseWorkspace.withSuffix(getFilePathSuffix() + "script").child(CHECKOUT_DIR_KEY.mac(scm.getKey()));
     }
 
     private String getFilePathSuffix() {

@@ -375,8 +375,9 @@ private void logInterpolationWarnings(String stepName, @CheckForNull ArgumentsAc
             return;
         }
 
+        final EnvVars nonNullEnvVars = envVars; // Workaround for NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE false positive in lambdas: https://github.com/spotbugs/spotbugs/issues/552.
         List<String> scanResults = sensitiveVariables.stream()
-                .filter(e -> !envVars.get(e, "").isEmpty() && interpolatedStrings.stream().anyMatch(g -> g.contains(envVars.get(e))))
+                .filter(e -> !nonNullEnvVars.get(e, "").isEmpty() && interpolatedStrings.stream().anyMatch(g -> g.contains(nonNullEnvVars.get(e))))
                 .collect(Collectors.toList());
 
         if (scanResults != null && !scanResults.isEmpty()) {

@@ -24,6 +24,7 @@
 
 package org.jenkinsci.plugins.workflow.cps;
 
+import hudson.Functions;
 import hudson.model.ParametersAction;
 import hudson.model.ParametersDefinitionProperty;
 import hudson.model.Result;
@@ -35,13 +36,21 @@
 import hudson.plugins.git.UserRemoteConfig;
 import hudson.plugins.git.extensions.GitSCMExtension;
 import hudson.scm.ChangeLogSet;
+import hudson.scm.SubversionSCM;
 import hudson.triggers.SCMTrigger;
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import jenkins.model.Jenkins;
 import jenkins.plugins.git.GitSampleRepoRule;
 import jenkins.plugins.git.GitStep;
+import jenkins.scm.impl.subversion.SubversionSampleRepoRule;
+import org.apache.commons.io.FileUtils;
 import org.jenkinsci.plugins.workflow.TestDurabilityHintProvider;
 import org.jenkinsci.plugins.workflow.actions.WorkspaceAction;
 import org.jenkinsci.plugins.workflow.flow.FlowDurabilityHint;
@@ -59,11 +68,18 @@
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.SingleFileSCM;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
+import static org.hamcrest.io.FileMatchers.anExistingFile;
+import static org.junit.Assume.assumeFalse;
+
 public class CpsScmFlowDefinitionTest {
 
     @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();
     @Rule public JenkinsRule r = new JenkinsRule();
     @Rule public GitSampleRepoRule sampleRepo = new GitSampleRepoRule();
+    @Rule public SubversionSampleRepoRule sampleRepoSvn = new SubversionSampleRepoRule();
     @Rule public GitSampleRepoRule invalidRepo = new GitSampleRepoRule();
 
     @Test public void configRoundtrip() throws Exception {
@@ -241,4 +257,50 @@ public class CpsScmFlowDefinitionTest {
         r.assertLogContains("version one", r.assertBuildStatusSuccess(p.scheduleBuild2(0)));
         r.assertLogContains("version two", r.assertBuildStatusSuccess(p.scheduleBuild2(0, new ParametersAction(new StringParameterValue("SCRIPT_PATH", "otherFlow.groovy")))));
     }
+
+    @Issue("SECURITY-2463")
+    @Test public void checkoutDirectoriesAreNotReusedByDifferentScms() throws Exception {
+        assumeFalse(Functions.isWindows()); // Checkout hook is not cross-platform.
+        sampleRepo.init();
+        sampleRepo.write("Jenkinsfile", "echo('git library')");
+        sampleRepo.git("add", "Jenkinsfile");
+        sampleRepo.git("commit", "--message=init");
+        sampleRepoSvn.init();
+        sampleRepoSvn.write("Jenkinsfile", "echo('subversion library')");
+        // Copy .git folder from the Git repo into the SVN repo as data.
+        File gitDirInSvnRepo = new File(sampleRepoSvn.wc(), ".git");
+        FileUtils.copyDirectory(new File(sampleRepo.getRoot(), ".git"), gitDirInSvnRepo);
+        String jenkinsRootDir = r.jenkins.getRootDir().toString();
+        // Add a Git post-checkout hook to the .git folder in the SVN repo.
+        Files.write(gitDirInSvnRepo.toPath().resolve("hooks/post-checkout"), ("#!/bin/sh\ntouch '" + jenkinsRootDir + "/hook-executed'\n").getBytes(StandardCharsets.UTF_8));
+        sampleRepoSvn.svnkit("add", sampleRepoSvn.wc() + "/Jenkinsfile");
+        sampleRepoSvn.svnkit("add", sampleRepoSvn.wc() + "/.git");
+        sampleRepoSvn.svnkit("propset", "svn:executable", "ON", sampleRepoSvn.wc() + "/.git/hooks/post-checkout");
+        sampleRepoSvn.svnkit("commit", "--message=init", sampleRepoSvn.wc());
+        // Run a build using the SVN repo.
+        WorkflowJob p = r.createProject(WorkflowJob.class);
+        p.setDefinition(new CpsScmFlowDefinition(new SubversionSCM(sampleRepoSvn.trunkUrl()), "Jenkinsfile"));
+        r.buildAndAssertSuccess(p);
+        // Run a build using the Git repo. It should be checked out to a different directory than the SVN repo.
+        p.setDefinition(new CpsScmFlowDefinition(new GitStep(sampleRepo.toString()).createSCM(), "Jenkinsfile"));
+        WorkflowRun b2 = r.buildAndAssertSuccess(p);
+        assertThat(new File(r.jenkins.getRootDir(), "hook-executed"), not(anExistingFile()));
+    }
+
+    @Issue("SECURITY-2595")
+    @Test
+    public void scriptPathSymlinksCannotEscapeCheckoutDirectory() throws Exception {
+        sampleRepo.init();
+        Path secrets = Paths.get(sampleRepo.getRoot().getPath(), "Jenkinsfile");
+        Files.createSymbolicLink(secrets, Paths.get(r.jenkins.getRootDir() + "/secrets/master.key"));
+        sampleRepo.git("add", ".");
+        sampleRepo.git("commit", "-m", "init");
+
+        WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
+        GitStep step = new GitStep(sampleRepo.toString());
+        p.setDefinition(new CpsScmFlowDefinition(step.createSCM(), "Jenkinsfile"));
+        WorkflowRun b = r.buildAndAssertStatus(Result.FAILURE, p);
+        assertThat(b.getExecution(), nullValue());
+        r.assertLogContains("Jenkinsfile references a file that is not inside " + r.jenkins.getWorkspaceFor(p), b);
+    }
 }