Skip to content

Commit b295606

Browse files
Pldi23Pldi23
committed
“SECURITY-2478”
1 parent 7881959 commit b295606

File tree

6 files changed

+147
-0
lines changed

6 files changed

+147
-0
lines changed

‎src/main/java/hudson/plugins/git/GitSCM.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import jenkins.model.Jenkins;
5353
import jenkins.plugins.git.GitSCMMatrixUtil;
5454
import jenkins.plugins.git.GitToolChooser;
55+
import jenkins.util.SystemProperties;
5556
import net.sf.json.JSONObject;
5657

5758
import org.eclipse.jgit.errors.MissingObjectException;
@@ -76,6 +77,8 @@
7677
import java.io.PrintStream;
7778
import java.io.Serializable;
7879
import java.io.Writer;
80+
import java.nio.file.Files;
81+
import java.nio.file.Paths;
7982
import java.text.MessageFormat;
8083
import java.util.AbstractList;
8184
import java.util.ArrayList;
@@ -85,6 +88,7 @@
8588
import java.util.HashSet;
8689
import java.util.Iterator;
8790
import java.util.List;
91+
import java.util.Locale;
8892
import java.util.Map;
8993
import java.util.Set;
9094
import java.util.logging.Level;
@@ -119,6 +123,11 @@
119123
*/
120124
public class GitSCM extends GitSCMBackwardCompatibility {
121125

126+
static final String ALLOW_LOCAL_CHECKOUT_PROPERTY = GitSCM.class.getName() + ".ALLOW_LOCAL_CHECKOUT";
127+
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL")
128+
public static /* not final */ boolean ALLOW_LOCAL_CHECKOUT =
129+
SystemProperties.getBoolean(ALLOW_LOCAL_CHECKOUT_PROPERTY);
130+
122131
/**
123132
* Store a config version so we're able to migrate config on various
124133
* functionality upgrades.
@@ -1269,6 +1278,10 @@ private boolean determineSecondFetch(CloneOption option, @NonNull RemoteConfig r
12691278
public void checkout(Run<?, ?> build, Launcher launcher, FilePath workspace, TaskListener listener, File changelogFile, SCMRevisionState baseline)
12701279
throws IOException, InterruptedException {
12711280

1281+
if (!ALLOW_LOCAL_CHECKOUT && !workspace.isRemote()) {
1282+
abortIfSourceIsLocal();
1283+
}
1284+
12721285
if (VERBOSE)
12731286
listener.getLogger().println("Using checkout strategy: " + getBuildChooser().getDisplayName());
12741287

@@ -1380,6 +1393,17 @@ public void checkout(Run<?, ?> build, Launcher launcher, FilePath workspace, Tas
13801393
}
13811394
}
13821395

1396+
private void abortIfSourceIsLocal() throws AbortException {
1397+
for (UserRemoteConfig userRemoteConfig: getUserRemoteConfigs()) {
1398+
String remoteUrl = userRemoteConfig.getUrl();
1399+
if (remoteUrl != null && (remoteUrl.toLowerCase(Locale.ENGLISH).startsWith("file://") || Files.exists(Paths.get(remoteUrl)))) {
1400+
throw new AbortException("Checkout of Git remote '" + remoteUrl + "' aborted because it references a local directory, " +
1401+
"which may be insecure. You can allow local checkouts anyway by setting the system property '" +
1402+
ALLOW_LOCAL_CHECKOUT_PROPERTY + "' to true.");
1403+
}
1404+
}
1405+
}
1406+
13831407
private void printCommitMessageToLog(TaskListener listener, GitClient git, final Build revToBuild)
13841408
throws IOException {
13851409
try {

‎src/test/java/hudson/plugins/git/Security2478Test.java‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package hudson.plugins.git;
2+
3+
import hudson.model.Result;
4+
import jenkins.plugins.git.GitSampleRepoRule;
5+
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
6+
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
7+
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
8+
import org.junit.After;
9+
import org.junit.Before;
10+
import org.junit.Rule;
11+
import org.junit.Test;
12+
import org.jvnet.hudson.test.Issue;
13+
import org.jvnet.hudson.test.JenkinsRule;
14+
15+
import java.io.File;
16+
17+
import static org.junit.Assert.assertFalse;
18+
19+
public class Security2478Test {
20+
21+
@Rule
22+
public JenkinsRule rule = new JenkinsRule();
23+
24+
@Rule
25+
public GitSampleRepoRule sampleRepo = new GitSampleRepoRule();
26+
27+
28+
@Before
29+
public void setUpAllowNonRemoteCheckout() {
30+
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
31+
}
32+
33+
@After
34+
public void disallowNonRemoteCheckout() {
35+
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
36+
}
37+
38+
@Issue("SECURITY-2478")
39+
@Test
40+
public void checkoutShouldNotAbortWhenLocalSourceAndRunningOnAgent() throws Exception {
41+
assertFalse("Non Remote checkout should be disallowed", GitSCM.ALLOW_LOCAL_CHECKOUT);
42+
rule.createOnlineSlave();
43+
sampleRepo.init();
44+
sampleRepo.write("file", "v1");
45+
sampleRepo.git("commit", "--all", "--message=test commit");
46+
WorkflowJob p = rule.jenkins.createProject(WorkflowJob.class, "pipeline");
47+
48+
String script = "node('slave0') {\n" +
49+
" checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [], userRemoteConfigs: [[url: '" + sampleRepo.fileUrl() + "', credentialsId: '']]])\n" +
50+
"}";
51+
p.setDefinition(new CpsFlowDefinition(script, true));
52+
WorkflowRun run = rule.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0));
53+
rule.assertLogNotContains("aborted because it references a local directory, which may be insecure. " +
54+
"You can allow local checkouts anyway by setting the system property 'hudson.plugins.git.GitSCM.ALLOW_LOCAL_CHECKOUT' to true.", run);
55+
}
56+
57+
@Issue("SECURITY-2478")
58+
@Test
59+
public void checkoutShouldAbortWhenSourceIsNonRemoteAndRunningOnController() throws Exception {
60+
assertFalse("Non Remote checkout should be disallowed", GitSCM.ALLOW_LOCAL_CHECKOUT);
61+
WorkflowJob p = rule.jenkins.createProject(WorkflowJob.class, "pipeline");
62+
String workspaceDir = rule.jenkins.getRootDir().getAbsolutePath();
63+
64+
String path = "file://" + workspaceDir + File.separator + "jobName@script" + File.separator + "anyhmachash";
65+
String escapedPath = path.replace("\\", "\\\\"); // for windows
66+
String script = "node {\n" +
67+
" checkout([$class: 'GitSCM', branches: [[name: '*/main']], extensions: [], userRemoteConfigs: [[" +
68+
"url: '" + escapedPath + "'," +
69+
" credentialsId: '']]])\n" +
70+
"}";
71+
p.setDefinition(new CpsFlowDefinition(script, true));
72+
WorkflowRun run = rule.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0));
73+
rule.assertLogContains("Checkout of Git remote '" + path + "' " +
74+
"aborted because it references a local directory, which may be insecure. " +
75+
"You can allow local checkouts anyway by setting the system property 'hudson.plugins.git.GitSCM.ALLOW_LOCAL_CHECKOUT' to true.", run);
76+
}
77+
}

‎src/test/java/hudson/plugins/git/extensions/GitSCMExtensionTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import hudson.plugins.git.GitSCM;
66
import hudson.plugins.git.TestGitRepo;
77
import hudson.util.StreamTaskListener;
8+
import org.junit.After;
89
import org.junit.Before;
910
import org.junit.ClassRule;
1011
import org.junit.Rule;
@@ -38,6 +39,16 @@ public void setUp() throws Exception {
3839
before();
3940
}
4041

42+
@Before
43+
public void allowNonRemoteCheckout() {
44+
GitSCM.ALLOW_LOCAL_CHECKOUT = true;
45+
}
46+
47+
@After
48+
public void disallowNonRemoteCheckout() {
49+
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
50+
}
51+
4152
protected abstract void before() throws Exception;
4253

4354
/**

‎src/test/java/hudson/plugins/git/extensions/impl/PruneStaleTagPipelineTest.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@
2828
import java.util.logging.Level;
2929
import java.util.logging.Logger;
3030

31+
import hudson.plugins.git.GitSCM;
3132
import org.apache.commons.io.FileUtils;
3233
import org.jenkinsci.plugins.gitclient.GitClient;
3334
import org.jenkinsci.plugins.gitclient.TestCliGitAPIImpl;
3435
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
3536
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
3637
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
38+
import org.junit.After;
3739
import org.junit.Assert;
3840
import org.junit.Before;
3941
import org.junit.Rule;
@@ -63,6 +65,16 @@ public void setup() throws Exception {
6365
listener = new LogTaskListener(Logger.getLogger("prune tags"), Level.FINEST);
6466
}
6567

68+
@Before
69+
public void allowNonRemoteCheckout() {
70+
GitSCM.ALLOW_LOCAL_CHECKOUT = true;
71+
}
72+
73+
@After
74+
public void disallowNonRemoteCheckout() {
75+
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
76+
}
77+
6678
@Issue("JENKINS-61869")
6779
@Test
6880
public void verify_that_local_tag_is_pruned_when_not_exist_on_remote_using_pipeline() throws Exception {

‎src/test/java/jenkins/plugins/git/GitSampleRepoRule.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.gargoylesoftware.htmlunit.util.NameValuePair;
2929
import hudson.Launcher;
3030
import hudson.model.TaskListener;
31+
import hudson.plugins.git.GitSCM;
3132
import hudson.util.StreamTaskListener;
3233
import java.io.ByteArrayOutputStream;
3334
import java.io.File;
@@ -48,6 +49,16 @@ public final class GitSampleRepoRule extends AbstractSampleDVCSRepoRule {
4849

4950
private static final Logger LOGGER = Logger.getLogger(GitSampleRepoRule.class.getName());
5051

52+
protected void before() throws Throwable {
53+
super.before();
54+
GitSCM.ALLOW_LOCAL_CHECKOUT = true;
55+
}
56+
57+
protected void after() {
58+
super.after();
59+
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
60+
}
61+
5162
public void git(String... cmds) throws Exception {
5263
run("git", cmds);
5364
}

‎src/test/java/org/jenkinsci/plugins/gittagmessage/AbstractGitTagMessageExtensionTest.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
import hudson.model.Job;
44
import hudson.model.Queue;
55
import hudson.model.Run;
6+
import hudson.plugins.git.GitSCM;
67
import hudson.plugins.git.util.BuildData;
78
import jenkins.model.ParameterizedJobMixIn;
89
import org.jenkinsci.plugins.gitclient.Git;
910
import org.jenkinsci.plugins.gitclient.GitClient;
11+
import org.junit.After;
1012
import org.junit.Before;
1113
import org.junit.Rule;
1214
import org.junit.Test;
@@ -48,6 +50,16 @@ public void setUp() throws IOException, InterruptedException {
4850
repo.init();
4951
}
5052

53+
@Before
54+
public void allowNonRemoteCheckout() {
55+
GitSCM.ALLOW_LOCAL_CHECKOUT = true;
56+
}
57+
58+
@After
59+
public void disallowNonRemoteCheckout() {
60+
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
61+
}
62+
5163
@Test
5264
public void commitWithoutTagShouldNotExportMessage() throws Exception {
5365
// Given a git repo without any tags

0 commit comments

Comments
 (0)