Skip to content

Commit

Permalink
“SECURITY-2478”
Browse files Browse the repository at this point in the history
  • Loading branch information
Pldi23 committed May 4, 2022
1 parent 7881959 commit b295606
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 0 deletions.
24 changes: 24 additions & 0 deletions src/main/java/hudson/plugins/git/GitSCM.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import jenkins.model.Jenkins;
import jenkins.plugins.git.GitSCMMatrixUtil;
import jenkins.plugins.git.GitToolChooser;
import jenkins.util.SystemProperties;
import net.sf.json.JSONObject;

import org.eclipse.jgit.errors.MissingObjectException;
Expand All @@ -76,6 +77,8 @@
import java.io.PrintStream;
import java.io.Serializable;
import java.io.Writer;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.text.MessageFormat;
import java.util.AbstractList;
import java.util.ArrayList;
Expand All @@ -85,6 +88,7 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.logging.Level;
Expand Down Expand Up @@ -119,6 +123,11 @@
*/
public class GitSCM extends GitSCMBackwardCompatibility {

static final String ALLOW_LOCAL_CHECKOUT_PROPERTY = GitSCM.class.getName() + ".ALLOW_LOCAL_CHECKOUT";
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL")
public static /* not final */ boolean ALLOW_LOCAL_CHECKOUT =
SystemProperties.getBoolean(ALLOW_LOCAL_CHECKOUT_PROPERTY);

/**
* Store a config version so we're able to migrate config on various
* functionality upgrades.
Expand Down Expand Up @@ -1269,6 +1278,10 @@ private boolean determineSecondFetch(CloneOption option, @NonNull RemoteConfig r
public void checkout(Run<?, ?> build, Launcher launcher, FilePath workspace, TaskListener listener, File changelogFile, SCMRevisionState baseline)
throws IOException, InterruptedException {

if (!ALLOW_LOCAL_CHECKOUT && !workspace.isRemote()) {
abortIfSourceIsLocal();
}

if (VERBOSE)
listener.getLogger().println("Using checkout strategy: " + getBuildChooser().getDisplayName());

Expand Down Expand Up @@ -1380,6 +1393,17 @@ public void checkout(Run<?, ?> build, Launcher launcher, FilePath workspace, Tas
}
}

private void abortIfSourceIsLocal() throws AbortException {
for (UserRemoteConfig userRemoteConfig: getUserRemoteConfigs()) {
String remoteUrl = userRemoteConfig.getUrl();
if (remoteUrl != null && (remoteUrl.toLowerCase(Locale.ENGLISH).startsWith("file://") || Files.exists(Paths.get(remoteUrl)))) {
throw new AbortException("Checkout of Git remote '" + remoteUrl + "' aborted because it references a local directory, " +
"which may be insecure. You can allow local checkouts anyway by setting the system property '" +
ALLOW_LOCAL_CHECKOUT_PROPERTY + "' to true.");
}
}
}

private void printCommitMessageToLog(TaskListener listener, GitClient git, final Build revToBuild)
throws IOException {
try {
Expand Down
77 changes: 77 additions & 0 deletions src/test/java/hudson/plugins/git/Security2478Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package hudson.plugins.git;

import hudson.model.Result;
import jenkins.plugins.git.GitSampleRepoRule;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

import java.io.File;

import static org.junit.Assert.assertFalse;

public class Security2478Test {

@Rule
public JenkinsRule rule = new JenkinsRule();

@Rule
public GitSampleRepoRule sampleRepo = new GitSampleRepoRule();


@Before
public void setUpAllowNonRemoteCheckout() {
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
}

@After
public void disallowNonRemoteCheckout() {
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
}

@Issue("SECURITY-2478")
@Test
public void checkoutShouldNotAbortWhenLocalSourceAndRunningOnAgent() throws Exception {
assertFalse("Non Remote checkout should be disallowed", GitSCM.ALLOW_LOCAL_CHECKOUT);
rule.createOnlineSlave();
sampleRepo.init();
sampleRepo.write("file", "v1");
sampleRepo.git("commit", "--all", "--message=test commit");
WorkflowJob p = rule.jenkins.createProject(WorkflowJob.class, "pipeline");

String script = "node('slave0') {\n" +
" checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [], userRemoteConfigs: [[url: '" + sampleRepo.fileUrl() + "', credentialsId: '']]])\n" +
"}";
p.setDefinition(new CpsFlowDefinition(script, true));
WorkflowRun run = rule.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0));
rule.assertLogNotContains("aborted because it references a local directory, which may be insecure. " +
"You can allow local checkouts anyway by setting the system property 'hudson.plugins.git.GitSCM.ALLOW_LOCAL_CHECKOUT' to true.", run);
}

@Issue("SECURITY-2478")
@Test
public void checkoutShouldAbortWhenSourceIsNonRemoteAndRunningOnController() throws Exception {
assertFalse("Non Remote checkout should be disallowed", GitSCM.ALLOW_LOCAL_CHECKOUT);
WorkflowJob p = rule.jenkins.createProject(WorkflowJob.class, "pipeline");
String workspaceDir = rule.jenkins.getRootDir().getAbsolutePath();

String path = "file://" + workspaceDir + File.separator + "jobName@script" + File.separator + "anyhmachash";
String escapedPath = path.replace("\\", "\\\\"); // for windows
String script = "node {\n" +
" checkout([$class: 'GitSCM', branches: [[name: '*/main']], extensions: [], userRemoteConfigs: [[" +
"url: '" + escapedPath + "'," +
" credentialsId: '']]])\n" +
"}";
p.setDefinition(new CpsFlowDefinition(script, true));
WorkflowRun run = rule.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0));
rule.assertLogContains("Checkout of Git remote '" + path + "' " +
"aborted because it references a local directory, which may be insecure. " +
"You can allow local checkouts anyway by setting the system property 'hudson.plugins.git.GitSCM.ALLOW_LOCAL_CHECKOUT' to true.", run);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import hudson.plugins.git.GitSCM;
import hudson.plugins.git.TestGitRepo;
import hudson.util.StreamTaskListener;
import org.junit.After;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Rule;
Expand Down Expand Up @@ -38,6 +39,16 @@ public void setUp() throws Exception {
before();
}

@Before
public void allowNonRemoteCheckout() {
GitSCM.ALLOW_LOCAL_CHECKOUT = true;
}

@After
public void disallowNonRemoteCheckout() {
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
}

protected abstract void before() throws Exception;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import hudson.plugins.git.GitSCM;
import org.apache.commons.io.FileUtils;
import org.jenkinsci.plugins.gitclient.GitClient;
import org.jenkinsci.plugins.gitclient.TestCliGitAPIImpl;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
Expand Down Expand Up @@ -63,6 +65,16 @@ public void setup() throws Exception {
listener = new LogTaskListener(Logger.getLogger("prune tags"), Level.FINEST);
}

@Before
public void allowNonRemoteCheckout() {
GitSCM.ALLOW_LOCAL_CHECKOUT = true;
}

@After
public void disallowNonRemoteCheckout() {
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
}

@Issue("JENKINS-61869")
@Test
public void verify_that_local_tag_is_pruned_when_not_exist_on_remote_using_pipeline() throws Exception {
Expand Down
11 changes: 11 additions & 0 deletions src/test/java/jenkins/plugins/git/GitSampleRepoRule.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.gargoylesoftware.htmlunit.util.NameValuePair;
import hudson.Launcher;
import hudson.model.TaskListener;
import hudson.plugins.git.GitSCM;
import hudson.util.StreamTaskListener;
import java.io.ByteArrayOutputStream;
import java.io.File;
Expand All @@ -48,6 +49,16 @@ public final class GitSampleRepoRule extends AbstractSampleDVCSRepoRule {

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

protected void before() throws Throwable {
super.before();
GitSCM.ALLOW_LOCAL_CHECKOUT = true;
}

protected void after() {
super.after();
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
}

public void git(String... cmds) throws Exception {
run("git", cmds);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
import hudson.model.Job;
import hudson.model.Queue;
import hudson.model.Run;
import hudson.plugins.git.GitSCM;
import hudson.plugins.git.util.BuildData;
import jenkins.model.ParameterizedJobMixIn;
import org.jenkinsci.plugins.gitclient.Git;
import org.jenkinsci.plugins.gitclient.GitClient;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -48,6 +50,16 @@ public void setUp() throws IOException, InterruptedException {
repo.init();
}

@Before
public void allowNonRemoteCheckout() {
GitSCM.ALLOW_LOCAL_CHECKOUT = true;
}

@After
public void disallowNonRemoteCheckout() {
GitSCM.ALLOW_LOCAL_CHECKOUT = false;
}

@Test
public void commitWithoutTagShouldNotExportMessage() throws Exception {
// Given a git repo without any tags
Expand Down

0 comments on commit b295606

Please # to comment.