Skip to content

Commit

Permalink
SECURITY-2478
Browse files Browse the repository at this point in the history
  • Loading branch information
@Pldi23
Pldi23 committed May 4, 2022
1 parent d2009c6 commit b995436
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 1 deletion.
25 changes: 24 additions & 1 deletion src/main/java/hudson/plugins/mercurial/MercurialSCM.java
View file
Expand Up @@ -11,6 +11,7 @@
import hudson.Extension;
import hudson.FilePath;
import hudson.Launcher;
import hudson.Main;
import hudson.Util;
import hudson.matrix.MatrixRun;
import hudson.model.AbstractBuild;
Expand Down Expand Up @@ -50,8 +51,11 @@
import java.io.Serializable;
import java.net.MalformedURLException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.logging.Level;
Expand All @@ -62,6 +66,7 @@
import javax.annotation.Nonnull;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.apache.commons.lang.StringUtils;
import org.ini4j.Ini;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
Expand All @@ -74,6 +79,11 @@
*/
public class MercurialSCM extends SCM implements Serializable {

static final String ALLOW_LOCAL_CHECKOUT_PROPERTY = MercurialSCM.class.getName() + ".ALLOW_LOCAL_CHECKOUT";
//TODO: use SystemProperties instead after jenkins version upgrade to 2.236
static /* not final */ boolean ALLOW_LOCAL_CHECKOUT =
Boolean.parseBoolean(System.getProperty(ALLOW_LOCAL_CHECKOUT_PROPERTY, String.valueOf(Main.isUnitTest)));

// Environment vars names to be exposed
private static final String ENV_MERCURIAL_REVISION = "MERCURIAL_REVISION";
private static final String ENV_MERCURIAL_REVISION_SHORT = "MERCURIAL_REVISION_SHORT";
Expand Down Expand Up @@ -561,6 +571,11 @@ public void checkout(Run<?, ?> build, Launcher launcher, FilePath workspace, fin
throws IOException, InterruptedException {

MercurialInstallation mercurialInstallation = findInstallation(installation);

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

final boolean jobShouldUseSharing = mercurialInstallation != null && mercurialInstallation.isUseSharing();

Node node = workspaceToNode(workspace);
Expand Down Expand Up @@ -596,7 +611,15 @@ public void checkout(Run<?, ?> build, Launcher launcher, FilePath workspace, fin
}
}
}


private void abortIfSourceLocal() throws IOException {
if (StringUtils.isNotEmpty(source) &&
(source.toLowerCase(Locale.ENGLISH).startsWith("file://") || Files.exists(Paths.get(source)))) {

throw new AbortException("Checkout of Mercurial source '" + source + "' 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 boolean canReuseWorkspace(FilePath repo, Node node,
boolean jobShouldUseSharing, Run<?,?> build,
Launcher launcher, TaskListener listener)
Expand Down
117 changes: 117 additions & 0 deletions src/test/java/hudson/plugins/mercurial/Security2478Test.java
View file
@@ -0,0 +1,117 @@
package hudson.plugins.mercurial;

import hudson.FilePath;
import hudson.model.Result;
import hudson.slaves.DumbSlave;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.rules.TestRule;
import org.jvnet.hudson.test.FlagRule;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

import java.io.File;
import java.util.Collections;

import static org.junit.Assert.assertFalse;

public class Security2478Test {

private static final String INSTALLATION = "mercurial";

@Rule
public JenkinsRule rule = new JenkinsRule();
@Rule
public MercurialRule m = new MercurialRule(rule);

@Rule
public TestRule notAllowNonRemoteCheckout = new FlagRule<>(() -> MercurialSCM.ALLOW_LOCAL_CHECKOUT, x -> MercurialSCM.ALLOW_LOCAL_CHECKOUT = x, false);

@Rule
public TemporaryFolder tmp = new TemporaryFolder();
private File repo;

@Before
public void setUp() throws Exception {
repo = tmp.getRoot();
rule.jenkins
.getDescriptorByType(MercurialInstallation.DescriptorImpl.class)
.setInstallations(new MercurialInstallation(INSTALLATION, "", "hg",
false, true, new File(tmp.newFolder(),"custom-dir").getAbsolutePath(), false, "",
Collections.emptyList()));

}

@Issue("SECURITY-2478")
@Test
public void checkoutShouldAbortWhenSourceIsNonRemoteAndBuildOnController() throws Exception {
assertFalse("Non Remote checkout should be disallowed", MercurialSCM.ALLOW_LOCAL_CHECKOUT);
WorkflowJob p = rule.jenkins.createProject(WorkflowJob.class, "pipeline");
FilePath sourcePath = rule.jenkins.getRootPath().createTempDir("t", "");
String script = "node {\n" +
"checkout([$class: 'MercurialSCM', credentialsId: '', installation: 'mercurial', source: '" + sourcePath + "'])\n" +
"}";
p.setDefinition(new CpsFlowDefinition(script, true));
WorkflowRun run = rule.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0));
rule.assertLogContains("Checkout of Mercurial source '" + sourcePath + "' aborted because it references a local directory, which may be insecure. You can allow local checkouts anyway by setting the system property '" + MercurialSCM.ALLOW_LOCAL_CHECKOUT_PROPERTY + "' to true.", run);
}

@Issue("SECURITY-2478")
@Test
public void checkoutOnAgentShouldNotAbortWhenSourceIsNonRemoteAndBuildOnAgent() throws Exception {
assertFalse("Non Remote checkout should be disallowed", MercurialSCM.ALLOW_LOCAL_CHECKOUT);
DumbSlave agent = rule.createOnlineSlave();
FilePath workspace = agent.getRootPath().createTempDir("t", "");
m.hg(workspace, "init");
m.touchAndCommit(workspace, "a");
WorkflowJob p = rule.jenkins.createProject(WorkflowJob.class, "pipeline");
String script = "node('slave0') {\n" +
"checkout([$class: 'MercurialSCM', credentialsId: '', installation: 'mercurial', source: '" + workspace + "'])\n" +
"}";
p.setDefinition(new CpsFlowDefinition(script, true));
rule.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0));
}

@Issue("SECURITY-2478")
@Test
public void checkoutShouldNotAbortWhenSourceIsAlias() throws Exception {
assertFalse("Non Remote checkout should be disallowed", MercurialSCM.ALLOW_LOCAL_CHECKOUT);

WorkflowJob p = rule.jenkins.createProject(WorkflowJob.class, "pipeline");
String aliasName = "alias1";
// configure mercurial installation with an alias in a path
rule.jenkins.getDescriptorByType(MercurialInstallation.DescriptorImpl.class).setInstallations(new MercurialInstallation("mercurial", "", "hg", false, false,"", false, "[paths]\n" + aliasName + " = https://www.mercurial-scm.org/repo/hello", null));
String script = "node {\n" +
"checkout([$class: 'MercurialSCM', credentialsId: '', installation: 'mercurial', source: '" + aliasName + "'])\n" +
"}";
p.setDefinition(new CpsFlowDefinition(script, true));
m.hg(new FilePath(repo), "init");
m.touchAndCommit(new FilePath(repo), "a");
WorkflowRun run = rule.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0));
rule.assertLogNotContains("Checkout of Mercurial source '" + aliasName + "' aborted because it references a local directory, which may be insecure. You can allow local checkouts anyway by setting the system property '" + MercurialSCM.ALLOW_LOCAL_CHECKOUT_PROPERTY + "' to true.", run);

}

@Issue("SECURITY-2478")
@Test
public void checkoutShouldNotAbortWhenSourceIsAliasPointingToLocalPath() throws Exception {
assertFalse("Non Remote checkout should be disallowed", MercurialSCM.ALLOW_LOCAL_CHECKOUT);

WorkflowJob p = rule.jenkins.createProject(WorkflowJob.class, "pipeline");
String aliasName = "alias1";
// configure mercurial installation with an alias in a path
rule.jenkins.getDescriptorByType(MercurialInstallation.DescriptorImpl.class).setInstallations(new MercurialInstallation("mercurial", "", "hg", false, false,"", false, "[paths]\n" + aliasName + " = " + repo.getPath(), null));
String script = "node {\n" +
"checkout([$class: 'MercurialSCM', credentialsId: '', installation: 'mercurial', source: 'alias1'])\n" +
"}";
p.setDefinition(new CpsFlowDefinition(script, true));
m.hg(new FilePath(repo), "init");
m.touchAndCommit(new FilePath(repo), "a");
rule.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0));
}
}

0 comments on commit b995436

Please sign in to comment.