SECURITY-1951

Author: dwnusbaum

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java

@@ -37,6 +37,7 @@
 import hudson.model.ItemGroup;
 import hudson.model.Run;
 import hudson.model.TaskListener;
+import hudson.scm.SCM;
 import hudson.security.AccessControlled;
 import java.io.File;
 import java.io.IOException;
@@ -60,6 +61,7 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import javax.inject.Inject;
 import jenkins.model.Jenkins;
+import jenkins.scm.impl.SingleSCMSource;
 import org.codehaus.groovy.control.MultipleCompilationErrorsException;
 import org.codehaus.groovy.runtime.InvokerHelper;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.AbstractWhitelist;
@@ -192,6 +194,18 @@ public static class Execution extends AbstractSynchronousNonBlockingStepExecutio
             } else if (version == null) {
                 throw new AbortException("Must specify a version for library " + name);
             }
+            // When a user specifies a non-null retriever, they may be using SCMVar in its configuration,
+            // so we need to run MultibranchScmRevisionVerifier to prevent unsafe behavior.
+            // SCMVar would typically be used with SCMRetriever, but it is also possible to use it with SCMSourceRetriever and SingleSCMSource.
+            // There may be false-positive rejections if a Multibranch Pipeline for the repo of a Pipeline library
+            // uses the library step with a non-null retriever to check out a static version of the library.
+            // Fixing this would require us being able to detect usage of SCMVar precisely, which is not currently possible.
+            else if (retriever instanceof SCMRetriever) {
+                verifyRevision(((SCMRetriever) retriever).getScm(), name);
+            } else if (retriever instanceof SCMSourceRetriever && ((SCMSourceRetriever) retriever).getScm() instanceof SingleSCMSource) {
+                verifyRevision(((SingleSCMSource) ((SCMSourceRetriever) retriever).getScm()).getScm(), name);
+            }
+
             LibraryRecord record = new LibraryRecord(name, version, trusted, changelog, cachingConfiguration, source);
             LibrariesAction action = run.getAction(LibrariesAction.class);
             if (action == null) {
@@ -219,6 +233,12 @@ public static class Execution extends AbstractSynchronousNonBlockingStepExecutio
             return new LoadedClasses(name, record.getDirectoryName(), trusted, changelog, run);
         }
 
+        private void verifyRevision(SCM scm, String name) throws IOException, InterruptedException {
+            for (LibraryStepRetrieverVerifier revisionVerifier : LibraryStepRetrieverVerifier.all()) {
+                revisionVerifier.verify(this.run, listener, scm, name);
+            }
+        }
+
     }
 
     public static final class LoadedClasses extends GroovyObjectSupport implements Serializable {

src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverVerifier.java renamed to src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStepRetrieverVerifier.java

@@ -11,11 +11,11 @@
 import java.io.IOException;
 
 @Restricted(NoExternalUse.class)
-public interface SCMSourceRetrieverVerifier extends ExtensionPoint {
+public interface LibraryStepRetrieverVerifier extends ExtensionPoint {
 
     void verify(Run<?, ?> run, TaskListener listener, SCM scm, String name) throws IOException, InterruptedException;
 
-    static ExtensionList<SCMSourceRetrieverVerifier> all() {
-        return ExtensionList.lookup(SCMSourceRetrieverVerifier.class);
+    static ExtensionList<LibraryStepRetrieverVerifier> all() {
+        return ExtensionList.lookup(LibraryStepRetrieverVerifier.class);
     }
 }

src/main/java/org/jenkinsci/plugins/workflow/libs/MultibranchScmRevisionVerifier.java

@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.workflow.libs;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.AbortException;
 import hudson.model.Job;
 import hudson.model.Run;
@@ -17,7 +18,10 @@
 import java.io.IOException;
 
 @OptionalExtension(requirePlugins={"workflow-multibranch"})
-public class MultibranchScmRevisionVerifier implements SCMSourceRetrieverVerifier {
+public class MultibranchScmRevisionVerifier implements LibraryStepRetrieverVerifier {
+
+    @SuppressFBWarnings("MS_SHOULD_BE_FINAL") // For script console and tests.
+    public static boolean DISABLED = Boolean.getBoolean(MultibranchScmRevisionVerifier.class.getName() + ".DISABLED");
 
     /**
      * Abort library retrieval if the specified build is from a Multibranch Pipeline configured to build the library's SCM and the revision being built is untrusted.
@@ -26,6 +30,9 @@ public class MultibranchScmRevisionVerifier implements SCMSourceRetrieverVerifie
      */
     @Override
     public void verify(Run<?, ?> run, TaskListener listener, SCM libraryScm, String name) throws IOException, InterruptedException {
+        if (DISABLED) {
+            return;
+        }
         // Adapted from ReadTrustedStep
         Job<?, ?> job = run.getParent();
         BranchJobProperty property = job.getProperty(BranchJobProperty.class);

src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java

@@ -174,9 +174,6 @@ private static <T> T retrySCMOperation(TaskListener listener, Callable<T> task)
 
     @SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE", justification = "apparently bogus complaint about redundant nullcheck in try-with-resources")
     static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String libraryPath, FilePath target, Run<?, ?> run, TaskListener listener) throws Exception {
-        for (SCMSourceRetrieverVerifier revisionVerifier : SCMSourceRetrieverVerifier.all()) {
-            revisionVerifier.verify(run, listener, scm, name);
-        }
         // Adapted from CpsScmFlowDefinition:
         SCMStep delegate = new GenericSCMStep(scm);
         delegate.setPoll(false); // TODO we have no API for determining if a given SCMHead is branch-like or tag-like; would we want to turn on polling if the former?

src/test/java/org/jenkinsci/plugins/workflow/libs/SCMRetrieverTest.java

@@ -124,6 +124,62 @@ public class SCMRetrieverTest {
         r.assertLogContains("Library '" + libraryName + "' has been modified in an untrusted revision", run);
     }
 
+    @Test public void libraryCanBeRetrievedStaticallyEvenWhenPipelineScmUntrusted() throws Exception {
+        sampleRepo.init();
+        sampleRepo.write("vars/greet.groovy", "def call(recipient) {echo(/hello from $recipient/)}");
+        sampleRepo.write("src/pkg/Clazz.groovy", "package pkg; class Clazz {static String whereAmI() {'master'}}");
+        sampleRepo.write("Jenkinsfile", "greet(pkg.Clazz.whereAmI())"); // Library loaded implicitly.
+        sampleRepo.git("add", "vars", "src", "Jenkinsfile");
+        sampleRepo.git("commit", "--message=init");
+
+        sampleRepo.git("checkout", "-b", "fork");
+        sampleRepo.write("src/pkg/Clazz.groovy", "package pkg; class Clazz {static String whereAmI() {'fork'}}");
+        sampleRepo.git("commit", "--all", "--message=branching");
+
+        WorkflowMultiBranchProject mp = r.jenkins.createProject(WorkflowMultiBranchProject.class, "mp");
+        String libraryName = "stuff";
+        LibraryConfiguration config = new LibraryConfiguration(libraryName, new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true)));
+        config.setDefaultVersion("master");
+        config.setImplicit(true);
+        GlobalLibraries.get().setLibraries(Collections.singletonList(config));
+
+        SCMSource warySource = new WarySource(sampleRepo.toString());
+        mp.getSourcesList().add(new BranchSource(warySource));
+        WorkflowJob job = WorkflowMultiBranchProjectTest.scheduleAndFindBranchProject(mp, "fork");
+        r.waitUntilNoActivity();
+        WorkflowRun run = job.getLastBuild();
+        // The fork is untrusted, but that doesn't matter because we are using stuff@master, which the untrusted user can't modify.
+        r.assertBuildStatus(Result.SUCCESS, run);
+        r.assertLogContains("hello from master", run);
+    }
+
+    @Issue("SECURITY-1951")
+    @Test public void libraryCantBeRetrievedWithoutVersionUsingScmSourceRetriever() throws Exception {
+        sampleRepo.init();
+        sampleRepo.write("vars/greet.groovy", "def call(recipient) {echo(/hello to $recipient/)}");
+        sampleRepo.write("src/pkg/Clazz.groovy", "package pkg; class Clazz {static String whereAmI() {'master'}}");
+        sampleRepo.write("Jenkinsfile", "def lib = library(identifier: 'stuff@master', retriever: modernSCM(fromScm(name: 'master', scm: scm))); greet(lib.pkg.Clazz.whereAmI())");
+        sampleRepo.git("add", "vars", "src", "Jenkinsfile");
+        sampleRepo.git("commit", "--message=init");
+
+        sampleRepo.git("checkout", "-b", "fork");
+        sampleRepo.write("src/pkg/Clazz.groovy", "package pkg; class Clazz {static String whereAmI() {'fork'}}");
+        sampleRepo.git("commit", "--all", "--message=branching");
+
+        WorkflowMultiBranchProject mp = r.jenkins.createProject(WorkflowMultiBranchProject.class, "mp");
+        String libraryName = "stuff";
+        mp.getProperties().add(new FolderLibraries(Collections.singletonList(new LibraryConfiguration(libraryName, new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true))))));
+
+        SCMSource warySource = new WarySource(sampleRepo.toString());
+        mp.getSourcesList().add(new BranchSource(warySource));
+        WorkflowJob job = WorkflowMultiBranchProjectTest.scheduleAndFindBranchProject(mp, "fork");
+        r.waitUntilNoActivity();
+        WorkflowRun run = job.getLastBuild();
+
+        r.assertBuildStatus(Result.FAILURE, run);
+        r.assertLogContains("Library '" + libraryName + "' has been modified in an untrusted revision", run);
+    }
+
     public static class WarySource extends GitSCMSource {
 
         public WarySource(String remote) {