diff --git a/pom.xml b/pom.xml index 7498ae6e..7e5167a5 100644 --- a/pom.xml +++ b/pom.xml @@ -78,6 +78,21 @@ apache-httpcomponents-client-4-api 4.5.5-3.0 + + org.owasp.esapi + esapi + 2.2.0.0 + + + org.slf4j + slf4j-api + + + org.apache.httpcomponents + httpclient + + + org.slf4j slf4j-api diff --git a/src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/StashCause.java b/src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/StashCause.java index 94cecf8e..af49e861 100644 --- a/src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/StashCause.java +++ b/src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/StashCause.java @@ -1,8 +1,12 @@ package stashpullrequestbuilder.stashpullrequestbuilder; import hudson.model.Cause; +import java.net.URI; +import java.net.URISyntaxException; import java.util.Map; import java.util.TreeMap; +import org.owasp.esapi.Encoder; +import org.owasp.esapi.reference.DefaultEncoder; /** Created by Nathan McCarthy */ public class StashCause extends Cause { @@ -123,18 +127,38 @@ public Map getEnvironmentVariables() { @Override public String getShortDescription() { - return "PR #" - + this.getPullRequestId() - + " " - + this.getPullRequestTitle() - + " "; + return "" + getEscapedDescription() + " "; + } + + String getEscapedUrl() { + return getEncoder().encodeForHTMLAttribute(getPrUrl().toASCIIString()); + } + + String getEscapedDescription() { + return getEncoder() + .encodeForHTML("PR #" + this.getPullRequestId() + " " + this.getPullRequestTitle()); + } + + URI getPrUrl() { + try { + return new URI(stashHost) + .resolve( + new URI( + null, + null, + "/projects/" + + this.getDestinationRepositoryOwner() + + "/repos/" + + this.getDestinationRepositoryName() + + "/pull-requests/" + + this.getPullRequestId(), + null)); + } catch (URISyntaxException e) { + throw new IllegalStateException(e); + } + } + + private static Encoder getEncoder() { + return DefaultEncoder.getInstance(); } } diff --git a/src/main/resources/ESAPI.properties b/src/main/resources/ESAPI.properties new file mode 100644 index 00000000..387509ef --- /dev/null +++ b/src/main/resources/ESAPI.properties @@ -0,0 +1 @@ +ESAPI.Encoder=org.owasp.esapi.reference.DefaultEncoder \ No newline at end of file diff --git a/src/test/java/stashpullrequestbuilder/stashpullrequestbuilder/StashCauseTest.java b/src/test/java/stashpullrequestbuilder/stashpullrequestbuilder/StashCauseTest.java new file mode 100644 index 00000000..f7de8d28 --- /dev/null +++ b/src/test/java/stashpullrequestbuilder/stashpullrequestbuilder/StashCauseTest.java @@ -0,0 +1,119 @@ +package stashpullrequestbuilder.stashpullrequestbuilder; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.object.HasToString.hasToString; +import static org.junit.Assert.assertThat; + +import org.junit.Test; + +public class StashCauseTest { + + @Test + public void short_description_doesnt_allow_injection() { + + StashCause stashCause = + new StashCause( + "", + "", + "", + "", + "", + PULL_REQUEST_ID, + OWNER, + DESTINATION_REPOSITORY_NAME, + PULL_REQUEST_TITLE, + "", + "", + "", + "", + null); + + assertThat( + stashCause.getShortDescription(), + is( + "" + + "PR #id<>&'" title<>&'" ")); + } + + @Test + public void will_produce_valid_PR_url() { + + StashCause stashCause = + new StashCause( + "https://stash.host", + "", + "", + "", + "", + PULL_REQUEST_ID, + OWNER, + DESTINATION_REPOSITORY_NAME, + PULL_REQUEST_TITLE, + "", + "", + "", + "", + null); + + assertThat( + stashCause.getPrUrl(), + hasToString( + "https://stash.host/projects/owner%3C%3E&'%22/repos/name%3C%3E&'%22/pull-requests/id%3C%3E&'%22")); + } + + @Test + public void will_escape_PR_url() { + + StashCause stashCause = + new StashCause( + "https://stash.host", + "", + "", + "", + "", + PULL_REQUEST_ID, + OWNER, + DESTINATION_REPOSITORY_NAME, + PULL_REQUEST_TITLE, + "", + "", + "", + "", + null); + + assertThat( + stashCause.getEscapedUrl(), + is( + "https://stash.host/projects/owner%3C%3E&'%22/repos/name%3C%3E&'%22/pull-requests/id%3C%3E&'%22")); + } + + @Test + public void will_escape_PR_description() { + + StashCause stashCause = + new StashCause( + "https://stash.host", + "", + "", + "", + "", + PULL_REQUEST_ID, + OWNER, + DESTINATION_REPOSITORY_NAME, + PULL_REQUEST_TITLE, + "", + "", + "", + "", + null); + + assertThat( + stashCause.getEscapedDescription(), + is("PR #id<>&'" title<>&'"")); + } + + public static final String OWNER = "owner<>&'\""; + public static final String DESTINATION_REPOSITORY_NAME = "name<>&'\""; + public static final String PULL_REQUEST_ID = "id<>&'\""; + public static final String PULL_REQUEST_TITLE = "title<>&'\""; +}