From b295606e0b865c298fde27bea14f9b7535a976e6 Mon Sep 17 00:00:00 2001 From: Dmitry_Platonov Date: Wed, 4 May 2022 10:48:37 +0200 Subject: [PATCH] =?UTF-8?q?=E2=80=9CSECURITY-2478=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/hudson/plugins/git/GitSCM.java | 24 ++++++ .../hudson/plugins/git/Security2478Test.java | 77 +++++++++++++++++++ .../git/extensions/GitSCMExtensionTest.java | 11 +++ .../impl/PruneStaleTagPipelineTest.java | 12 +++ .../plugins/git/GitSampleRepoRule.java | 11 +++ .../AbstractGitTagMessageExtensionTest.java | 12 +++ 6 files changed, 147 insertions(+) create mode 100644 src/test/java/hudson/plugins/git/Security2478Test.java diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java index aafbf2c6af..c8fbe43190 100644 --- a/src/main/java/hudson/plugins/git/GitSCM.java +++ b/src/main/java/hudson/plugins/git/GitSCM.java @@ -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; @@ -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; @@ -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; @@ -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. @@ -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()); @@ -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 { diff --git a/src/test/java/hudson/plugins/git/Security2478Test.java b/src/test/java/hudson/plugins/git/Security2478Test.java new file mode 100644 index 0000000000..7b56bb52e9 --- /dev/null +++ b/src/test/java/hudson/plugins/git/Security2478Test.java @@ -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); + } +} diff --git a/src/test/java/hudson/plugins/git/extensions/GitSCMExtensionTest.java b/src/test/java/hudson/plugins/git/extensions/GitSCMExtensionTest.java index e5e868dd9d..06a4d5490a 100644 --- a/src/test/java/hudson/plugins/git/extensions/GitSCMExtensionTest.java +++ b/src/test/java/hudson/plugins/git/extensions/GitSCMExtensionTest.java @@ -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; @@ -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; /** diff --git a/src/test/java/hudson/plugins/git/extensions/impl/PruneStaleTagPipelineTest.java b/src/test/java/hudson/plugins/git/extensions/impl/PruneStaleTagPipelineTest.java index 62612ee96a..2fda95897e 100644 --- a/src/test/java/hudson/plugins/git/extensions/impl/PruneStaleTagPipelineTest.java +++ b/src/test/java/hudson/plugins/git/extensions/impl/PruneStaleTagPipelineTest.java @@ -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; @@ -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 { diff --git a/src/test/java/jenkins/plugins/git/GitSampleRepoRule.java b/src/test/java/jenkins/plugins/git/GitSampleRepoRule.java index 656ac16922..c935ad3da4 100644 --- a/src/test/java/jenkins/plugins/git/GitSampleRepoRule.java +++ b/src/test/java/jenkins/plugins/git/GitSampleRepoRule.java @@ -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; @@ -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); } diff --git a/src/test/java/org/jenkinsci/plugins/gittagmessage/AbstractGitTagMessageExtensionTest.java b/src/test/java/org/jenkinsci/plugins/gittagmessage/AbstractGitTagMessageExtensionTest.java index 12a599a388..4f8be39664 100644 --- a/src/test/java/org/jenkinsci/plugins/gittagmessage/AbstractGitTagMessageExtensionTest.java +++ b/src/test/java/org/jenkinsci/plugins/gittagmessage/AbstractGitTagMessageExtensionTest.java @@ -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; @@ -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