Skip to content

Commit

Permalink
[SECURITY-1651] Protect BuildLogIndication doMatchText
Browse files Browse the repository at this point in the history
    Permission check and require post.
  • Loading branch information
rsandell committed Dec 16, 2019
1 parent 81acb23 commit f316c88
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.sonyericsson.jenkins.plugins.bfa.Messages;
import com.sonyericsson.jenkins.plugins.bfa.PluginImpl;
import com.sonyericsson.jenkins.plugins.bfa.model.BuildLogFailureReader;
import com.sonyericsson.jenkins.plugins.bfa.model.FailureReader;
import hudson.Extension;
Expand All @@ -44,6 +45,7 @@
import jenkins.model.Jenkins;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.interceptor.RequirePOST;

import java.io.IOException;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -248,10 +250,12 @@ public String getDisplayName() {
* the string does not match the pattern,
* {@link FormValidation#error(java.lang.String) } otherwise.
*/
@RequirePOST
public FormValidation doMatchText(
@QueryParameter("pattern") final String testPattern,
@QueryParameter("testText") String testText,
@QueryParameter("textSourceIsUrl") final boolean textSourceIsUrl) {
Jenkins.get().checkPermission(PluginImpl.UPDATE_PERMISSION);
if (textSourceIsUrl) {
testText = testText.replaceAll("/\\./", "/").replaceAll("/view/change-requests", "");
Matcher urlMatcher = URL_PATTERN.matcher(testText);
Expand Down Expand Up @@ -332,7 +336,9 @@ && isValidBuildId(urlParts[2])) {
return FormValidation.error(Messages.InvalidURL_Error());
} else {
try {
if (testText.matches(testPattern)) {
final Pattern pattern = Pattern.compile(testPattern);
final Matcher matcher = pattern.matcher(new FailureReader.InterruptibleCharSequence(testText));
if (matcher.matches()) {
return FormValidation.ok(Messages.StringMatchesPattern());
}
return FormValidation.warning(Messages.StringDoesNotMatchPattern());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@
*/
package com.sonyericsson.jenkins.plugins.bfa.model.indication;

import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import com.gargoylesoftware.htmlunit.HttpMethod;
import com.gargoylesoftware.htmlunit.Page;
import com.gargoylesoftware.htmlunit.WebRequest;
import com.gargoylesoftware.htmlunit.util.UrlUtils;
import com.sonyericsson.jenkins.plugins.bfa.Messages;
import com.sonyericsson.jenkins.plugins.bfa.PluginImpl;
import com.sonyericsson.jenkins.plugins.bfa.model.BuildLogFailureReader;
import com.sonyericsson.jenkins.plugins.bfa.model.FailureCause;
import com.sonyericsson.jenkins.plugins.bfa.model.FailureReader;
Expand All @@ -39,21 +45,30 @@
import hudson.model.Cause;
import hudson.model.FreeStyleBuild;
import hudson.model.FreeStyleProject;
import hudson.model.Item;
import hudson.model.Result;
import hudson.security.SecurityRealm;
import hudson.util.FormValidation;
import jenkins.model.Jenkins;
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 org.jvnet.hudson.test.MockBuilder;

import java.io.BufferedReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;

import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;

/**
* Tests for the BuildLogIndication.
Expand Down Expand Up @@ -276,4 +291,91 @@ public void testDoMatchTextUrlInvalid() {
assertEquals(Messages.InvalidURL_Error(), formValidation.getMessage());
assertEquals(FormValidation.Kind.ERROR, formValidation.kind);
}

// CS IGNORE MagicNumber FOR NEXT 200 LINES. REASON: test data.

@Test @Issue("SECURITY-1651")
public void testDoMatchNotHttpGetAccessible() throws Exception {
lockDown();
final JenkinsRule.WebClient webClient = j.createWebClient();
webClient.assertFails("descriptorByName/"
+ BuildLogIndication.class.getName()
+ "/matchText?"
+ "pattern=[a-z]+&"
+ "testText=hello&"
+ "textSourceIsUrl=false",
405);
}

@Test @Issue("SECURITY-1651")
public void testDoMatchHttpPostAccessible() throws Exception {
lockDown();
final JenkinsRule.WebClient webClient = j.createWebClient();
post(webClient, "descriptorByName/"
+ BuildLogIndication.class.getName()
+ "/matchText?"
+ "pattern=[a-z]+&"
+ "testText=hello&"
+ "textSourceIsUrl=false",
null, 403);
}

@Test @Issue("SECURITY-1651")
public void testDoMatchHttpPostAccessibleWithPermission() throws Exception {
lockDown();
final JenkinsRule.WebClient webClient = j.createWebClient().login("bob");
post(webClient, "descriptorByName/"
+ BuildLogIndication.class.getName()
+ "/matchText?"
+ "pattern=[a-z]+&"
+ "testText=hello&"
+ "textSourceIsUrl=false",
null, null);
}

/**
* Performs an HTTP POST request to the relative url.
*
* @param webClient the client
* @param relative the url relative to the context path
* @param expectedContentType if expecting specific content type or null if not
* @param expectedStatus if expecting a failing http status code or null if not
* @throws IOException if so
*/
private static void post(JenkinsRule.WebClient webClient, String relative,
String expectedContentType, Integer expectedStatus) throws IOException {
WebRequest request = new WebRequest(
UrlUtils.toUrlUnsafe(webClient.getContextPath() + relative),
HttpMethod.POST);
try {
Page p = webClient.getPage(request);
if (expectedContentType != null) {
assertThat(p.getWebResponse().getContentType(), is(expectedContentType));
}
} catch (FailingHttpStatusCodeException e) {
if (expectedStatus != null) {
assertEquals(expectedStatus.intValue(), e.getStatusCode());
} else {
throw e;
}
}
}

/**
* Lock down the instance.
*/
private void lockDown() {
SecurityRealm securityRealm = j.createDummySecurityRealm();
j.getInstance().setSecurityRealm(securityRealm);
j.getInstance().setAuthorizationStrategy(
new MockAuthorizationStrategy().grant(Jenkins.READ).everywhere().toAuthenticated());
j.jenkins.setCrumbIssuer(null); //Not really testing csrf right now
j.getInstance().setAuthorizationStrategy(
new MockAuthorizationStrategy().grant(Item.READ, Item.DISCOVER).everywhere().toAuthenticated()
.grant(PluginImpl.VIEW_PERMISSION).everywhere().toAuthenticated()
.grant(Jenkins.READ, Item.DISCOVER).everywhere().toEveryone()
.grant(Item.CONFIGURE).everywhere().to("bob")
.grant(PluginImpl.UPDATE_PERMISSION).everywhere().to("bob")
.grant(Jenkins.ADMINISTER).everywhere().to("alice"));
}
}

0 comments on commit f316c88

Please sign in to comment.