Skip to content

Commit 9521045

Browse files
Pldi23dwnusbaum
Pldi23
authored and
dwnusbaum
committed
[SECURITY-1951]
1 parent e62a4eb commit 9521045

File tree

5 files changed

+152
-15
lines changed

5 files changed

+152
-15
lines changed

‎pom.xml

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -124,31 +124,36 @@
124124
<artifactId>workflow-support</artifactId>
125125
</dependency>
126126

127-
<!-- test only plugins -->
128127
<dependency>
129-
<groupId>org.jenkins-ci.plugins.workflow</groupId>
130-
<artifactId>workflow-support</artifactId>
131-
<classifier>tests</classifier>
132-
<scope>test</scope>
128+
<groupId>org.jenkins-ci.plugins</groupId>
129+
<artifactId>variant</artifactId>
133130
</dependency>
134131
<dependency>
135132
<groupId>org.jenkins-ci.plugins.workflow</groupId>
136-
<artifactId>workflow-job</artifactId>
137-
<scope>test</scope>
133+
<artifactId>workflow-multibranch</artifactId>
134+
<optional>true</optional>
135+
</dependency>
136+
<dependency>
137+
<groupId>org.jenkins-ci.plugins</groupId>
138+
<artifactId>branch-api</artifactId>
139+
<optional>true</optional>
138140
</dependency>
141+
142+
<!-- test only plugins -->
139143
<dependency>
140144
<groupId>org.jenkins-ci.plugins.workflow</groupId>
141-
<artifactId>workflow-basic-steps</artifactId>
145+
<artifactId>workflow-support</artifactId>
146+
<classifier>tests</classifier>
142147
<scope>test</scope>
143148
</dependency>
144149
<dependency>
145150
<groupId>org.jenkins-ci.plugins.workflow</groupId>
146-
<artifactId>workflow-durable-task-step</artifactId>
151+
<artifactId>workflow-basic-steps</artifactId>
147152
<scope>test</scope>
148153
</dependency>
149154
<dependency>
150155
<groupId>org.jenkins-ci.plugins.workflow</groupId>
151-
<artifactId>workflow-multibranch</artifactId>
156+
<artifactId>workflow-durable-task-step</artifactId>
152157
<scope>test</scope>
153158
</dependency>
154159
<dependency>
@@ -203,10 +208,5 @@
203208
<version>1.10.1</version>
204209
<scope>test</scope>
205210
</dependency>
206-
<dependency>
207-
<groupId>org.jenkins-ci.plugins</groupId>
208-
<artifactId>branch-api</artifactId>
209-
<scope>test</scope>
210-
</dependency>
211211
</dependencies>
212212
</project>

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package org.jenkinsci.plugins.workflow.libs;
2+
3+
import hudson.AbortException;
4+
import hudson.model.Job;
5+
import hudson.model.Run;
6+
import hudson.model.TaskListener;
7+
import hudson.scm.SCM;
8+
import jenkins.branch.Branch;
9+
import jenkins.scm.api.SCMHead;
10+
import jenkins.scm.api.SCMRevision;
11+
import jenkins.scm.api.SCMRevisionAction;
12+
import jenkins.scm.api.SCMSource;
13+
import jenkins.scm.api.SCMSourceOwner;
14+
import org.jenkinsci.plugins.variant.OptionalExtension;
15+
import org.jenkinsci.plugins.workflow.multibranch.BranchJobProperty;
16+
17+
import java.io.IOException;
18+
19+
@OptionalExtension(requirePlugins={"workflow-multibranch"})
20+
public class MultibranchScmRevisionVerifier implements SCMSourceRetrieverVerifier {
21+
22+
/**
23+
* 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.
24+
* Comparable to the defenses against untrusted users in {@code SCMBinder}, but here we care about the library rather than the Jenkinsfile.
25+
* @throws AbortException if the specified build is from a Multibranch Pipeline configured to build the library's SCM and the revision being built is untrusted
26+
*/
27+
@Override
28+
public void verify(Run<?, ?> run, TaskListener listener, SCM libraryScm, String name) throws IOException, InterruptedException {
29+
// Adapted from ReadTrustedStep
30+
Job<?, ?> job = run.getParent();
31+
BranchJobProperty property = job.getProperty(BranchJobProperty.class);
32+
if (property == null || !(job.getParent() instanceof SCMSourceOwner)) {
33+
// Not a multibranch project, so we do not care.
34+
// It is possible to use legacySCM(scm) from a non-multibranch Pipeline that uses CpsScmFlowDefinition,
35+
// but in that case we implicitly trust the changes because only a user with Item/Configure permission can select which branches to build.
36+
return;
37+
}
38+
Branch pipelineBranch = property.getBranch();
39+
SCMSource pipelineScmSource = ((SCMSourceOwner)job.getParent()).getSCMSource(pipelineBranch.getSourceId());
40+
if (pipelineScmSource == null) {
41+
throw new IllegalStateException(pipelineBranch.getSourceId() + " not found");
42+
}
43+
SCMHead head = pipelineBranch.getHead();
44+
SCMRevision headRevision;
45+
SCMRevisionAction action = run.getAction(SCMRevisionAction.class);
46+
if (action != null) {
47+
headRevision = action.getRevision();
48+
} else {
49+
headRevision = pipelineScmSource.fetch(head, listener);
50+
if (headRevision == null) {
51+
throw new AbortException("Could not determine exact tip revision of " + pipelineBranch.getName());
52+
}
53+
run.addAction(new SCMRevisionAction(pipelineScmSource, headRevision));
54+
}
55+
SCMRevision trustedRevision = pipelineScmSource.getTrustedRevision(headRevision, listener);
56+
if (!headRevision.equals(trustedRevision) && libraryScm.getKey().equals(pipelineScmSource.build(head, headRevision).getKey())) {
57+
throw new AbortException("Library '" + name + "' has been modified in an untrusted revision");
58+
}
59+
}
60+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ private static <T> T retrySCMOperation(TaskListener listener, Callable<T> task)
174174

175175
@SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE", justification = "apparently bogus complaint about redundant nullcheck in try-with-resources")
176176
static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String libraryPath, FilePath target, Run<?, ?> run, TaskListener listener) throws Exception {
177+
for (SCMSourceRetrieverVerifier revisionVerifier : SCMSourceRetrieverVerifier.all()) {
178+
revisionVerifier.verify(run, listener, scm, name);
179+
}
177180
// Adapted from CpsScmFlowDefinition:
178181
SCMStep delegate = new GenericSCMStep(scm);
179182
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/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverVerifier.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package org.jenkinsci.plugins.workflow.libs;
2+
3+
import hudson.ExtensionList;
4+
import hudson.ExtensionPoint;
5+
import hudson.model.Run;
6+
import hudson.model.TaskListener;
7+
import hudson.scm.SCM;
8+
import org.kohsuke.accmod.Restricted;
9+
import org.kohsuke.accmod.restrictions.NoExternalUse;
10+
11+
import java.io.IOException;
12+
13+
@Restricted(NoExternalUse.class)
14+
public interface SCMSourceRetrieverVerifier extends ExtensionPoint {
15+
16+
void verify(Run<?, ?> run, TaskListener listener, SCM scm, String name) throws IOException, InterruptedException;
17+
18+
static ExtensionList<SCMSourceRetrieverVerifier> all() {
19+
return ExtensionList.lookup(SCMSourceRetrieverVerifier.class);
20+
}
21+
}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,18 @@
2424

2525
package org.jenkinsci.plugins.workflow.libs;
2626

27+
import java.io.IOException;
2728
import java.util.Collections;
29+
import edu.umd.cs.findbugs.annotations.NonNull;
30+
import hudson.model.Result;
31+
import hudson.model.TaskListener;
2832
import jenkins.branch.BranchSource;
2933
import jenkins.plugins.git.GitSCMSource;
3034
import jenkins.plugins.git.GitSampleRepoRule;
3135
import jenkins.plugins.git.traits.BranchDiscoveryTrait;
36+
import jenkins.scm.api.SCMHead;
37+
import jenkins.scm.api.SCMRevision;
38+
import jenkins.scm.api.SCMSource;
3239
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
3340
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
3441
import org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProject;
@@ -38,6 +45,7 @@
3845
import org.junit.Test;
3946
import org.junit.Rule;
4047
import org.jvnet.hudson.test.BuildWatcher;
48+
import org.jvnet.hudson.test.Issue;
4149
import org.jvnet.hudson.test.JenkinsRule;
4250
import org.jvnet.hudson.test.Url;
4351

@@ -89,4 +97,49 @@ public class SCMRetrieverTest {
8997
r.assertLogContains("hello to earth", r.waitForCompletion(m1));
9098
}
9199

100+
@Issue("SECURITY-1951")
101+
@Test public void untrustedUsersCanOverideLibraryWithOtherSource() throws Exception {
102+
sampleRepo.init();
103+
sampleRepo.write("vars/greet.groovy", "def call(recipient) {echo(/hello to $recipient/)}");
104+
sampleRepo.write("src/pkg/Clazz.groovy", "package pkg; class Clazz {static String whereAmI() {'master'}}");
105+
sampleRepo.write("Jenkinsfile", "def lib = library identifier: 'stuff@snapshot', retriever: legacySCM(scm); greet(lib.pkg.Clazz.whereAmI())");
106+
sampleRepo.git("add", "vars", "src", "Jenkinsfile");
107+
sampleRepo.git("commit", "--message=init");
108+
109+
sampleRepo.git("checkout", "-b", "fork");
110+
sampleRepo.write("src/pkg/Clazz.groovy", "package pkg; class Clazz {static String whereAmI() {'fork'}}");
111+
sampleRepo.git("commit", "--all", "--message=branching");
112+
113+
WorkflowMultiBranchProject mp = r.jenkins.createProject(WorkflowMultiBranchProject.class, "mp");
114+
String libraryName = "stuff";
115+
mp.getProperties().add(new FolderLibraries(Collections.singletonList(new LibraryConfiguration(libraryName, new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true))))));
116+
117+
118+
SCMSource warySource = new WarySource(sampleRepo.toString());
119+
mp.getSourcesList().add(new BranchSource(warySource));
120+
WorkflowJob job = WorkflowMultiBranchProjectTest.scheduleAndFindBranchProject(mp, "fork");
121+
r.waitUntilNoActivity();
122+
WorkflowRun run = job.getLastBuild();
123+
r.assertBuildStatus(Result.FAILURE, run);
124+
r.assertLogContains("Library '" + libraryName + "' has been modified in an untrusted revision", run);
125+
}
126+
127+
public static class WarySource extends GitSCMSource {
128+
129+
public WarySource(String remote) {
130+
super(null, remote, "", "*", "", false);
131+
}
132+
@Override
133+
@NonNull
134+
public SCMRevision getTrustedRevision(@NonNull SCMRevision revision, @NonNull TaskListener listener) throws IOException, InterruptedException {
135+
String branch = revision.getHead().getName();
136+
if (branch.equals("master")) {
137+
return revision;
138+
} else {
139+
listener.getLogger().println("not trusting " + branch);
140+
return fetch(new SCMHead("master"), listener);
141+
}
142+
}
143+
}
144+
92145
}

0 commit comments

Comments
 (0)