forked from bklarson/Jenkins-repo-plugin
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
5 changed files
with
278 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| /* | ||
| * The MIT License | ||
| * | ||
| * Copyright (c) 2010, Brad Larson | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a copy | ||
| * of this software and associated documentation files (the "Software"), to deal | ||
| * in the Software without restriction, including without limitation the rights | ||
| * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
| * copies of the Software, and to permit persons to whom the Software is | ||
| * furnished to do so, subject to the following conditions: | ||
| * | ||
| * The above copyright notice and this permission notice shall be included in | ||
| * all copies or substantial portions of the Software. | ||
| * | ||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
| * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
| * THE SOFTWARE. | ||
| */ | ||
|
|
||
| package hudson.plugins.repo; | ||
|
|
||
| import hudson.AbortException; | ||
| import jenkins.util.xml.XMLUtils; | ||
| import org.w3c.dom.Document; | ||
| import org.w3c.dom.NamedNodeMap; | ||
| import org.w3c.dom.NodeList; | ||
|
|
||
| import org.xml.sax.SAXException; | ||
|
|
||
| import java.io.ByteArrayInputStream; | ||
| import java.io.IOException; | ||
| import java.util.Locale; | ||
|
|
||
| /** | ||
| * to validate manifest xml file and abort when remote references a local path. | ||
| */ | ||
| public final class ManifestValidator { | ||
|
|
||
| private ManifestValidator() { | ||
| // to hide the implicit public constructor | ||
| } | ||
|
|
||
| /** | ||
| * to validate manifest xml file and abort when remote references a local path. | ||
| * @param manifestText byte representation of manifest file | ||
| * @param manifestRepositoryUrl url | ||
| * @throws IOException when remote references a local path. | ||
| */ | ||
| public static void validate(final byte[] manifestText, final String manifestRepositoryUrl) | ||
| throws IOException { | ||
| if (manifestText.length > 0) { | ||
| try { | ||
| Document doc = XMLUtils.parse(new ByteArrayInputStream(manifestText)); | ||
| NodeList remote = doc.getElementsByTagName("remote"); | ||
| for (int i = 0; i < remote.getLength(); i++) { | ||
| NamedNodeMap attributes = remote.item(i).getAttributes(); | ||
| for (int j = 0; j < attributes.getLength(); j++) { | ||
| if ("fetch".equals(attributes.item(j).getNodeName()) | ||
| && attributes.item(j).getNodeValue() | ||
| .toLowerCase(Locale.ENGLISH).startsWith("file://")) { | ||
| // we don't need to check source using Files.exists because fetch | ||
| // attribute could resolve only local paths starting from 'file://' | ||
| throw new AbortException("Checkout of Repo url '" | ||
| + manifestRepositoryUrl | ||
| + "' aborted because manifest references a local " | ||
| + "directory, which may be insecure. You can allow " | ||
| + "local checkouts anyway" | ||
| + " by setting the system property '" | ||
| + RepoScm.ALLOW_LOCAL_CHECKOUT_PROPERTY + "' to true."); | ||
| } | ||
| } | ||
| } | ||
| } catch (SAXException e) { | ||
| throw new IOException("Could not validate manifest"); | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
56 changes: 56 additions & 0 deletions
56
src/test/java/hudson/plugins/repo/ManifestValidatorTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package hudson.plugins.repo; | ||
|
|
||
| import org.junit.Test; | ||
| import org.jvnet.hudson.test.Issue; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.charset.StandardCharsets; | ||
|
|
||
| import static org.hamcrest.MatcherAssert.assertThat; | ||
| import static org.hamcrest.Matchers.is; | ||
| import static org.junit.Assert.fail; | ||
|
|
||
| public class ManifestValidatorTest { | ||
|
|
||
| @Issue("SECURITY-2478") | ||
| @Test | ||
| public void validateWhenFetchAttributeReferencesLocalPathThenAbort() { | ||
| String manifest = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" + | ||
| "<manifest>\n" + | ||
| " <remote name=\"local\"\n" + | ||
| " fetch=\"file:///Users/d.platonov/workdir/\"\n" + | ||
| " revision=\"master\"\n" + | ||
| " review=\"\" />\n" + | ||
| "\n" + | ||
| " <project name=\"localProject\" path=\"localProject\" groups=\"lib\" remote=\"local\" />\n" + | ||
| "</manifest>"; | ||
| try { | ||
| ManifestValidator.validate(manifest.getBytes(StandardCharsets.UTF_8), "repoUrl"); | ||
| fail("should fail because fetch attribute in remote tag references a local path"); | ||
| } catch (IOException e) { | ||
| assertThat(e.getMessage(), is("Checkout of Repo url 'repoUrl' aborted because manifest references a local directory, " + | ||
| "which may be insecure. You can allow local checkouts anyway by setting the system property '" + | ||
| RepoScm.ALLOW_LOCAL_CHECKOUT_PROPERTY + "' to true.")); | ||
| } | ||
| } | ||
|
|
||
| @Issue("SECURITY-2478") | ||
| @Test | ||
| public void validateWhenValidManifestThenDoNotAbort() { | ||
| String manifest = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" + | ||
| "<manifest>\n" + | ||
| " <remote name=\"origin\"\n" + | ||
| " fetch=\"..\"\n" + // https://stackoverflow.com/questions/18251358/repo-manifest-xml-what-does-the-fetch-mean | ||
| " revision=\"master\"\n" + | ||
| " review=\"https://github.com\" />\n" + | ||
| "\n" + | ||
| " <project name=\"any\" path=\"any\" groups=\"gr\" remote=\"origin\" />\n" + | ||
| " </manifest>"; | ||
|
|
||
| try { | ||
| ManifestValidator.validate(manifest.getBytes(StandardCharsets.UTF_8), "repoUrl"); | ||
| } catch (Exception e) { | ||
| fail("fail because input is valid and no exception expected"); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| package hudson.plugins.repo; | ||
|
|
||
| import hudson.model.FreeStyleBuild; | ||
| import hudson.model.FreeStyleProject; | ||
| import hudson.model.Result; | ||
| import hudson.slaves.DumbSlave; | ||
| import org.junit.Rule; | ||
| import org.junit.Test; | ||
| import org.junit.rules.TemporaryFolder; | ||
| import org.jvnet.hudson.test.Issue; | ||
| import org.jvnet.hudson.test.JenkinsRule; | ||
|
|
||
| public class Security2478Test { | ||
|
|
||
| @Rule | ||
| public JenkinsRule rule = new JenkinsRule(); | ||
|
|
||
| @Rule | ||
| public TemporaryFolder testFolder = new TemporaryFolder(); | ||
|
|
||
| @Issue("SECURITY-2478") | ||
| @Test | ||
| public void checkoutShouldAbortWhenUrlIsNonRemoteAndBuildOnController() throws Exception { | ||
| FreeStyleProject freeStyleProject = rule.createFreeStyleProject(); | ||
| String manifestRepositoryUrl = testFolder.newFolder().toString(); | ||
| RepoScm scm = new RepoScm(manifestRepositoryUrl); | ||
| freeStyleProject.setScm(scm); | ||
| FreeStyleBuild freeStyleBuild = rule.assertBuildStatus(Result.FAILURE, freeStyleProject.scheduleBuild2(0)); | ||
| rule.assertLogContains("Checkout of Repo url '" + manifestRepositoryUrl + | ||
| "' aborted because it references a local directory, " + | ||
| "which may be insecure. You can allow local checkouts anyway by setting the system property '" + | ||
| RepoScm.ALLOW_LOCAL_CHECKOUT_PROPERTY + "' to true.", freeStyleBuild); | ||
| } | ||
|
|
||
| @Issue("SECURITY-2478") | ||
| @Test | ||
| public void checkoutShouldNotAbortWhenUrlIsNonRemoteAndEscapeHatchTrue() throws Exception { | ||
| try { | ||
| RepoScm.ALLOW_LOCAL_CHECKOUT = true; | ||
| FreeStyleProject freeStyleProject = rule.createFreeStyleProject(); | ||
| String manifestRepositoryUrl = testFolder.newFolder().toString(); | ||
| RepoScm scm = new RepoScm(manifestRepositoryUrl); | ||
| freeStyleProject.setScm(scm); | ||
| FreeStyleBuild freeStyleBuild = rule.assertBuildStatus(Result.FAILURE, freeStyleProject.scheduleBuild2(0)); | ||
|
|
||
| // build fails because of manifestRepositoryUrl is not a repo(git) repository, but we don't care, | ||
| // we verify that build was not aborted because of RepoScm uses local path. | ||
| rule.assertLogNotContains("Checkout of Repo url '" + manifestRepositoryUrl + | ||
| "' aborted because it references a local directory, " + | ||
| "which may be insecure. You can allow local checkouts anyway by setting the system property '" + | ||
| RepoScm.ALLOW_LOCAL_CHECKOUT_PROPERTY + "' to true.", freeStyleBuild); | ||
| } finally { | ||
| RepoScm.ALLOW_LOCAL_CHECKOUT = false; | ||
| } | ||
| } | ||
|
|
||
| @Issue("SECURITY-2478") | ||
| @Test | ||
| public void checkoutShouldNotAbortWhenUrlIsNonRemoteAndBuildOnAgent() throws Exception { | ||
| DumbSlave agent = rule.createOnlineSlave(); | ||
| FreeStyleProject freeStyleProject = rule.createFreeStyleProject(); | ||
|
|
||
| String manifestRepositoryUrl = testFolder.newFolder().toString(); | ||
|
|
||
| RepoScm scm = new RepoScm(manifestRepositoryUrl); | ||
| freeStyleProject.setScm(scm); | ||
| freeStyleProject.setAssignedLabel(agent.getSelfLabel()); | ||
|
|
||
| // build fails because of manifestRepositoryUrl is not a repo(git) repository, but we don't care, | ||
| // we verify that build was not aborted because of RepoScm uses local path. | ||
| rule.assertLogNotContains("Checkout of Repo url '" + manifestRepositoryUrl + | ||
| "' aborted because it references a local directory, " + | ||
| "which may be insecure. You can allow local checkouts anyway by setting the system property '" + | ||
| RepoScm.ALLOW_LOCAL_CHECKOUT_PROPERTY + "' to true.", freeStyleProject.scheduleBuild2(0).get()); | ||
| } | ||
| } |