Skip to content

Commit

Permalink
SECURITY-2205,2206,2207,2208,2764
Browse files Browse the repository at this point in the history
  • Loading branch information
bzzitsme authored and Pldi23 committed Jul 18, 2022
1 parent 1d1888e commit 7ba4a55
Show file tree
Hide file tree
Showing 11 changed files with 455 additions and 1 deletion.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ build/

# locally stored credentials
test-keys.txt

.DS_Store
7 changes: 7 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,13 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>4.6.1</version>
<scope>test</scope>
</dependency>

</dependencies>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ public final Charset getCharset() {
* Sends out the raw console output.
*/
public void doDeployText(StaplerRequest req, StaplerResponse rsp) throws IOException {
owner.getParent().checkPermission(DEPLOY);
rsp.setContentType("text/plain;charset=UTF-8");
// Prevent jelly from flushing stream so Content-Length header can be added afterwards
FlushProofOutputStream out = new FlushProofOutputStream(rsp.getCompressedOutputStream(req));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.cloudbees.plugins.deployer.sources;

import hudson.FilePath;

import java.io.File;
import java.io.IOException;

public class FilePathValidator {

private FilePathValidator() {
// to hide the implicit public constructor
}

/**
* Checks whether a given child path is a descendant of a given parent path using {@link File#getCanonicalFile}.
*
* If the child path does not exist, this method will canonicalize path elements such as {@code /../} and
* {@code /./} before comparing it to the parent path, and it will not throw an exception. If the child path
* does exist, symlinks will be resolved before checking whether the child is a descendant of the parent path.
* @param child FilePath
* @param parent FilePath
* @return boolean value of whether child path is a descendant of parent path
* @throws IllegalStateException when child or parent FilePath represent remote file
* @throws IOException when {@link File#getCanonicalFile} throws
*/
public static boolean isDescendant(FilePath child, FilePath parent) throws IOException {
if (child.isRemote() || parent.isRemote()) {
throw new IllegalStateException("Directory path '" + parent + "' is not located on the controller");
}
return new File(child.getRemote()).getCanonicalFile().toPath().startsWith(new File(parent.getRemote()).getCanonicalPath());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import hudson.FilePath;
import hudson.RelativePath;
import hudson.model.AbstractProject;
import hudson.model.Item;
import hudson.model.Job;
import hudson.model.Run;
import hudson.util.FormValidation;
Expand Down Expand Up @@ -197,18 +198,25 @@ public FormValidation doCheckDirectoryPath(@QueryParameter @RelativePath("..") f
@QueryParameter final String targetDescriptorId,
@QueryParameter final String value)
throws IOException, ServletException, InterruptedException {
Job job = findJob();
if (job != null) {
job.checkPermission(Item.WORKSPACE);
}
if (StringUtils.isEmpty(value)) {
return FormValidation.warning("You really should specify a directory, otherwise '.' is assumed");
}
if (Boolean.valueOf(fromWorkspace)) {
Job job = findJob();
if (job != null && job instanceof AbstractProject) {
FilePath someWorkspace = ((AbstractProject) job).getSomeWorkspace();
if (someWorkspace == null) {
return FormValidation.warning("The workspace is empty. Unable to validate '" + value + "'.");
}

FilePath dirPath = someWorkspace.child(value);

if (!FilePathValidator.isDescendant(dirPath, someWorkspace)) {
return FormValidation.error("Directory path '" + value + "' is not contained within the workspace for " + job.getDisplayName());
}
if (dirPath.exists()) {
if (dirPath.isDirectory()) {
return delegatePathValidationToTarget(value, targetDescriptorId, dirPath);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,20 @@ public String getFilePath() {
public File getApplicationFile(@NonNull Run run) {
if (run.getArtifactsDir().isDirectory()) {
File file = new File(run.getArtifactsDir(), filePath);
try {
if (!FilePathValidator.isDescendant(new FilePath(file), new FilePath(run.getArtifactsDir()))) {
throw new IllegalArgumentException("Directory path '" + filePath + "' is not contained within the artifacts directory for " + run.getDisplayName());
}
} catch (IOException e) {
throw new RuntimeException(e);
}
return file.exists() ? file : null;
} else {
return null;
}
}


/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -178,6 +186,11 @@ public FormValidation doCheckFilePath(@QueryParameter final String value)
"No artifacts were archived in the last successful run, unable to validate '" + value
+ "'");
}
FilePath artifactsDir = new FilePath(run.getArtifactsDir());
FilePath childDir = new FilePath(artifactsDir, value);
if (!FilePathValidator.isDescendant(childDir, artifactsDir)) {
return FormValidation.error("Directory path '" + value + "' is not contained within the artifacts directory for " + run.getFullDisplayName());
}
if (new File(run.getArtifactsDir(), value).isFile()) {
return FormValidation.ok();
}
Expand All @@ -195,6 +208,7 @@ public ListBoxModel doFillFilePathItems()

if (run != null) {
FileSet fileSet = new FileSet();
fileSet.setFollowSymlinks(false);
fileSet.setProject(new Project());
fileSet.setDir(run.getArtifactsDir());
fileSet.setIncludes("**/*.war");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import hudson.FilePath;
import hudson.RelativePath;
import hudson.model.AbstractProject;
import hudson.model.Item;
import hudson.model.Job;
import hudson.model.Run;
import hudson.util.FormValidation;
Expand Down Expand Up @@ -89,6 +90,7 @@ public File getApplicationFile(@NonNull Run run) {
File result = null;
if (run.getArtifactsDir().isDirectory()) {
FileSet fileSet = new FileSet();
fileSet.setFollowSymlinks(false);
fileSet.setProject(new Project());
fileSet.setDir(run.getArtifactsDir());
fileSet.setIncludes(getFilePattern());
Expand Down Expand Up @@ -200,6 +202,7 @@ public FormValidation doCheckFilePattern(@QueryParameter @RelativePath("..") fin
if (Boolean.valueOf(fromWorkspace)) {
Job job = findJob();
if (job != null && job instanceof AbstractProject) {
job.checkPermission(Item.WORKSPACE);
FilePath someWorkspace = ((AbstractProject) job).getSomeWorkspace();
if (someWorkspace == null) {
return FormValidation.warning("The workspace is empty. Unable to validate '" + value + "'.");
Expand All @@ -209,6 +212,9 @@ public FormValidation doCheckFilePattern(@QueryParameter @RelativePath("..") fin
return FormValidation.warning("Multiple files in the workspace match '" + value + "'");
}
if (filePaths.length == 1) {
if (!FilePathValidator.isDescendant(filePaths[0], someWorkspace)) {
return FormValidation.error("Directory path '" + value + "' is not contained within the workspace for " + job.getDisplayName());
}
return delegatePathValidationToTarget(value, targetDescriptorId, filePaths[0]);
}
}
Expand All @@ -219,6 +225,7 @@ public FormValidation doCheckFilePattern(@QueryParameter @RelativePath("..") fin
return FormValidation.error("There are no archived artifacts");
}
FileSet fileSet = new FileSet();
fileSet.setFollowSymlinks(false);
fileSet.setProject(new Project());
fileSet.setDir(run.getArtifactsDir());
fileSet.setIncludes(value);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package com.cloudbees.plugins.deployer;

import com.gargoylesoftware.htmlunit.Page;
import hudson.model.FreeStyleProject;
import hudson.model.Item;
import hudson.model.Result;
import jenkins.model.Jenkins;
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 org.jvnet.hudson.test.MockAuthorizationStrategy;

import java.io.File;
import java.net.HttpURLConnection;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

public class DeployNowRunActionTest {

@Rule
public JenkinsRule r = new JenkinsRule();

@Issue("SECURITY-2205")
@Test
public void doDeployTextWhenUserWithoutPermissionThenShouldReturnStatusForbidden() throws Exception {
FreeStyleProject project = r.createFreeStyleProject("test");
r.assertBuildStatus(Result.SUCCESS, project.scheduleBuild2(1));

JenkinsRule.WebClient webClient = r.createWebClient().withThrowExceptionOnFailingStatusCode(false);

webClient.login("user");
Page page = webClient.goTo("job/" + project.getName() + "/" + project.getLastSuccessfulBuild().getNumber() + "/deploy-now/deployText");
assertThat(page.getWebResponse().getStatusCode(), is(HttpURLConnection.HTTP_FORBIDDEN));
}

@Issue("SECURITY-2205")
@Test
public void doDeployTextWhenUserWithDeployPermissionThenShouldReturnOk() throws Exception {
FreeStyleProject project = r.createFreeStyleProject("test1");
r.assertBuildStatus(Result.SUCCESS, project.scheduleBuild2(1));
File logFile = new File(project.getLastSuccessfulBuild().getRootDir() + "/cloudbees-deploy-now.log");
logFile.createNewFile();

JenkinsRule.WebClient webClient = r.createWebClient().withThrowExceptionOnFailingStatusCode(false);
webClient.login("admin");
Page page = webClient.goTo("job/" + project.getName() + "/" + project.getLastSuccessfulBuild().getNumber() + "/deploy-now/deployText", "text/plain");
assertThat(page.getWebResponse().getStatusCode(), is(HttpURLConnection.HTTP_OK));
}

@Before
public void setUpAuthorization() {
r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
.grant(Jenkins.ADMINISTER, DeployNowRunAction.DEPLOY).everywhere().to("admin")
.grant(Jenkins.READ, Item.READ).everywhere().to("user"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package com.cloudbees.plugins.deployer.sources;

import com.gargoylesoftware.htmlunit.Page;
import hudson.Util;
import hudson.model.FreeStyleProject;
import hudson.model.Item;
import hudson.model.Result;
import hudson.model.TaskListener;
import jenkins.model.Jenkins;
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 org.jvnet.hudson.test.MockAuthorizationStrategy;

import java.io.File;
import java.net.HttpURLConnection;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;

public class FixedDirectoryDeploySourceTest {

@Rule
public JenkinsRule r = new JenkinsRule();

private FreeStyleProject project;

@Issue("SECURITY-2206")
@Test
public void doCheckDirectoryPathWhenUserWithoutPermissionThenStatusForbidden() throws Exception {
project = r.createFreeStyleProject();

JenkinsRule.WebClient webClient = r.createWebClient().withThrowExceptionOnFailingStatusCode(false);
webClient.login("user");
Page page = webClient.goTo("job/" + project.getName() +"/descriptorByName/com.cloudbees.plugins.deployer.sources.FixedDirectoryDeploySource/checkDirectoryPath?fromWorkspace=true&value=value");

assertThat(page.getWebResponse().getStatusCode(), is(HttpURLConnection.HTTP_FORBIDDEN));
}

@Issue("SECURITY-2206")
@Test
public void doCheckDirectoryPathWhenPathTraversalThenReturnError() throws Exception {
project = r.createFreeStyleProject();
r.assertBuildStatus(Result.SUCCESS, project.scheduleBuild2(1));

JenkinsRule.WebClient webClient = r.createWebClient().withThrowExceptionOnFailingStatusCode(false);
webClient.login("admin");
Page page = webClient.goTo("job/" + project.getName() +"/descriptorByName/com.cloudbees.plugins.deployer.sources.FixedDirectoryDeploySource/checkDirectoryPath?fromWorkspace=true&value=../../secret");

assertThat(page.getWebResponse().getContentAsString(), containsString("Directory path &#039;../../secret&#039; is not contained within the workspace for"));
}

@Issue("SECURITY-2206")
@Test
public void doCheckDirectoryPathWhenValueIsSymlinkThenReturnError() throws Exception {
project = r.createFreeStyleProject();
r.assertBuildStatus(Result.SUCCESS, project.scheduleBuild2(1));
Util.createSymlink(new File(project.getSomeWorkspace().getRemote()), r.jenkins.getRootDir().getAbsolutePath(), "temp_link", TaskListener.NULL);

JenkinsRule.WebClient webClient = r.createWebClient().withThrowExceptionOnFailingStatusCode(false);
webClient.login("admin");
Page page = webClient.goTo("job/" + project.getName() +"/descriptorByName/com.cloudbees.plugins.deployer.sources.FixedDirectoryDeploySource/checkDirectoryPath?fromWorkspace=true&value=temp_link");

assertThat(page.getWebResponse().getContentAsString(), containsString("Directory path &#039;temp_link&#039; is not contained within the workspace for"));
}

@Issue("SECURITY-2206")
@Test
public void doCheckDirectoryPathWhenParamsValidThenReturnOk() throws Exception {
project = r.createFreeStyleProject();
r.assertBuildStatus(Result.SUCCESS, project.scheduleBuild2(1));

project.getSomeWorkspace().child("test").mkdirs();

JenkinsRule.WebClient webClient = r.createWebClient().withThrowExceptionOnFailingStatusCode(false);
webClient.login("admin");
Page page = webClient.goTo("job/" + project.getName() +"/descriptorByName/com.cloudbees.plugins.deployer.sources.FixedDirectoryDeploySource/checkDirectoryPath?fromWorkspace=true&value=test");

assertThat(page.getWebResponse().getStatusCode(), is(HttpURLConnection.HTTP_OK));
assertThat(page.getWebResponse().getContentAsString(), is("<div/>"));
}

@Before
public void setUpAuthorization() {
r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
.grant(Jenkins.ADMINISTER).everywhere().to("admin")
.grant(Jenkins.READ, Item.READ).everywhere().to("user"));
}
}
Loading

0 comments on commit 7ba4a55

Please sign in to comment.