From 38ce5ce7cda90dc298109d45fa46eb9f3e69682b Mon Sep 17 00:00:00 2001 From: Wadeck Follonier Date: Sun, 21 Feb 2021 21:30:53 +0100 Subject: [PATCH] Merge back security tests (#5288) --- ...curity2147Test.java => Digester2Test.java} | 24 +- ...urity2025GraphTest.java => GraphTest.java} | 2 +- .../cli/CreateNodeCommandSEC2021Test.java | 72 ---- .../hudson/cli/CreateNodeCommandTest.java | 23 ++ .../cli/UpdateNodeCommandSEC2021Test.java | 77 ---- .../hudson/cli/UpdateNodeCommandTest.java | 26 ++ .../MarkupFormatterSecurity2153Test.java | 47 --- .../hudson/markup/MarkupFormatterTest.java | 34 ++ .../ComputerConfigDotXmlSEC2021Test.java | 52 --- .../model/ComputerConfigDotXmlTest.java | 34 ++ .../hudson/model/ComputerSEC1923Test.java | 81 ----- .../test/java/hudson/model/ComputerTest.java | 58 +++ .../hudson/model/FingerprintSEC2023Test.java | 262 -------------- .../java/hudson/model/FingerprintTest.java | 226 +++++++++++- .../model/FreeStyleProjectSEC1923Test.java | 80 ----- .../hudson/model/FreeStyleProjectTest.java | 60 ++++ .../model/ItemGroupMixInSEC1923Test.java | 81 ----- .../java/hudson/model/ItemGroupMixInTest.java | 57 +++ .../java/hudson/model/ViewSEC1923Test.java | 187 ---------- .../java/hudson/model/ViewSEC2171Test.java | 214 ----------- test/src/test/java/hudson/model/ViewTest.java | 336 ++++++++++++++++++ .../jenkins/model/JenkinsSEC2047Test.java | 83 ----- .../test/java/jenkins/model/JenkinsTest.java | 62 ++++ ...stSEC2035Test.java => HeteroListTest.java} | 5 +- .../ProtectedRootAction/index.jelly | 0 .../RootActionImpl/index.jelly | 0 26 files changed, 935 insertions(+), 1248 deletions(-) rename core/src/test/java/hudson/util/{Digester2Security2147Test.java => Digester2Test.java} (72%) rename core/src/test/java/hudson/util/{Security2025GraphTest.java => GraphTest.java} (98%) delete mode 100644 test/src/test/java/hudson/cli/CreateNodeCommandSEC2021Test.java delete mode 100644 test/src/test/java/hudson/cli/UpdateNodeCommandSEC2021Test.java delete mode 100644 test/src/test/java/hudson/markup/MarkupFormatterSecurity2153Test.java delete mode 100644 test/src/test/java/hudson/model/ComputerConfigDotXmlSEC2021Test.java delete mode 100644 test/src/test/java/hudson/model/ComputerSEC1923Test.java delete mode 100644 test/src/test/java/hudson/model/FingerprintSEC2023Test.java delete mode 100644 test/src/test/java/hudson/model/FreeStyleProjectSEC1923Test.java delete mode 100644 test/src/test/java/hudson/model/ItemGroupMixInSEC1923Test.java delete mode 100644 test/src/test/java/hudson/model/ViewSEC1923Test.java delete mode 100644 test/src/test/java/hudson/model/ViewSEC2171Test.java delete mode 100644 test/src/test/java/jenkins/model/JenkinsSEC2047Test.java rename test/src/test/java/lib/form/{HeteroListSEC2035Test.java => HeteroListTest.java} (98%) rename test/src/test/resources/jenkins/model/{JenkinsSEC2047Test => JenkinsTest}/ProtectedRootAction/index.jelly (100%) rename test/src/test/resources/lib/form/{HeteroListSEC2035Test => HeteroListTest}/RootActionImpl/index.jelly (100%) diff --git a/core/src/test/java/hudson/util/Digester2Security2147Test.java b/core/src/test/java/hudson/util/Digester2Test.java similarity index 72% rename from core/src/test/java/hudson/util/Digester2Security2147Test.java rename to core/src/test/java/hudson/util/Digester2Test.java index 3af5ee93b4..84def757ea 100644 --- a/core/src/test/java/hudson/util/Digester2Security2147Test.java +++ b/core/src/test/java/hudson/util/Digester2Test.java @@ -30,28 +30,38 @@ import org.xml.sax.SAXParseException; import java.io.FileNotFoundException; import java.net.ConnectException; +import java.util.Locale; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; -public class Digester2Security2147Test { +public class Digester2Test { private static final String RESOURCE_FILE_NAME = "Digester2Security2147TestData.xml"; @Issue("SECURITY-2147") @Test public void testProtection() throws Exception { + Locale defaultLocale = Locale.getDefault(); try { - new Digester2().parse(Digester2Security2147Test.class.getResourceAsStream(RESOURCE_FILE_NAME)); - Assert.fail("expected exception"); - } catch (SAXParseException ex) { - Assert.assertThat(ex.getMessage(), containsString("DOCTYPE is disallowed")); + // otherwise it's not passing on machine not running english as OS language + Locale.setDefault(Locale.ENGLISH); + try { + new Digester2().parse(Digester2Test.class.getResourceAsStream(RESOURCE_FILE_NAME)); + Assert.fail("expected exception"); + } catch (SAXParseException ex) { + assertThat(ex.getMessage(), containsString("DOCTYPE is disallowed")); + } + } + finally { + Locale.setDefault(defaultLocale); } } @Issue("SECURITY-2147") @Test public void testUnsafeBehavior() throws Exception { try { - new Digester2(false).parse(Digester2Security2147Test.class.getResourceAsStream(RESOURCE_FILE_NAME)); + new Digester2(false).parse(Digester2Test.class.getResourceAsStream(RESOURCE_FILE_NAME)); Assert.fail("expected exception"); } catch (FileNotFoundException|ConnectException ex) { // network or file access is bad @@ -64,7 +74,7 @@ public class Digester2Security2147Test { final String key = Digester2.class.getName() + ".UNSAFE"; try { System.setProperty(key, "true"); - new Digester2().parse(Digester2Security2147Test.class.getResourceAsStream(RESOURCE_FILE_NAME)); + new Digester2().parse(Digester2Test.class.getResourceAsStream(RESOURCE_FILE_NAME)); Assert.fail("expected exception"); } catch (FileNotFoundException|ConnectException ex) { // network or file access is bad diff --git a/core/src/test/java/hudson/util/Security2025GraphTest.java b/core/src/test/java/hudson/util/GraphTest.java similarity index 98% rename from core/src/test/java/hudson/util/Security2025GraphTest.java rename to core/src/test/java/hudson/util/GraphTest.java index ed4f01c9c3..8b82df3a98 100644 --- a/core/src/test/java/hudson/util/Security2025GraphTest.java +++ b/core/src/test/java/hudson/util/GraphTest.java @@ -28,7 +28,7 @@ import org.junit.Test; import java.awt.*; -public class Security2025GraphTest { +public class GraphTest { public static final int DEFAULT_W = 400; public static final int DEFAULT_H = 300; diff --git a/test/src/test/java/hudson/cli/CreateNodeCommandSEC2021Test.java b/test/src/test/java/hudson/cli/CreateNodeCommandSEC2021Test.java deleted file mode 100644 index 8c2dd00825..0000000000 --- a/test/src/test/java/hudson/cli/CreateNodeCommandSEC2021Test.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * The MIT License - * - * Copyright 2013 Red Hat, Inc. - * - * 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.cli; - -import hudson.model.Computer; -import hudson.model.Messages; -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 static hudson.cli.CLICommandInvoker.Matcher.failedWith; -import static hudson.cli.CLICommandInvoker.Matcher.hasNoStandardOutput; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.junit.Assert.assertEquals; - -//TODO to be merged into base test after security release -public class CreateNodeCommandSEC2021Test { - - private CLICommandInvoker command; - - @Rule public final JenkinsRule j = new JenkinsRule(); - - @Before public void setUp() { - - command = new CLICommandInvoker(j, new CreateNodeCommand()); - } - - @Test - @Issue("SECURITY-2021") - public void createNodeShouldFailIfNodeIsNotGood() throws Exception { - int nodeListSizeBefore = j.jenkins.getNodes().size(); - - final CLICommandInvoker.Result result = command - .authorizedTo(Computer.CREATE, Jenkins.READ) - .withStdin(CreateNodeCommandSEC2021Test.class.getResourceAsStream("node_sec2021.xml")) - .invoke() - ; - - assertThat(result.stderr(), containsString(Messages.Hudson_UnsafeChar('/'))); - assertThat(result, hasNoStandardOutput()); - assertThat(result, failedWith(1)); - - // ensure not side effects - assertEquals(nodeListSizeBefore, j.jenkins.getNodes().size()); - } -} diff --git a/test/src/test/java/hudson/cli/CreateNodeCommandTest.java b/test/src/test/java/hudson/cli/CreateNodeCommandTest.java index 0ae5fc30be..d7b0a32e27 100644 --- a/test/src/test/java/hudson/cli/CreateNodeCommandTest.java +++ b/test/src/test/java/hudson/cli/CreateNodeCommandTest.java @@ -31,7 +31,10 @@ import static org.hamcrest.Matchers.nullValue; import static hudson.cli.CLICommandInvoker.Matcher.failedWith; import static hudson.cli.CLICommandInvoker.Matcher.hasNoStandardOutput; import static hudson.cli.CLICommandInvoker.Matcher.succeededSilently; +import static org.junit.Assert.assertEquals; + import hudson.model.Computer; +import hudson.model.Messages; import hudson.model.Node; import hudson.model.Slave; import jenkins.model.Jenkins; @@ -39,6 +42,7 @@ 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; public class CreateNodeCommandTest { @@ -145,4 +149,23 @@ public class CreateNodeCommandTest { assertThat(result, hasNoStandardOutput()); assertThat(result, failedWith(4)); } + + @Test + @Issue("SECURITY-2021") + public void createNodeShouldFailIfNodeIsNotGood() { + int nodeListSizeBefore = j.jenkins.getNodes().size(); + + final CLICommandInvoker.Result result = command + .authorizedTo(Computer.CREATE, Jenkins.READ) + .withStdin(CreateNodeCommandTest.class.getResourceAsStream("node_sec2021.xml")) + .invoke() + ; + + assertThat(result.stderr(), containsString(Messages.Hudson_UnsafeChar('/'))); + assertThat(result, hasNoStandardOutput()); + assertThat(result, failedWith(1)); + + // ensure not side effects + assertEquals(nodeListSizeBefore, j.jenkins.getNodes().size()); + } } diff --git a/test/src/test/java/hudson/cli/UpdateNodeCommandSEC2021Test.java b/test/src/test/java/hudson/cli/UpdateNodeCommandSEC2021Test.java deleted file mode 100644 index 5a2d11cc7a..0000000000 --- a/test/src/test/java/hudson/cli/UpdateNodeCommandSEC2021Test.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * The MIT License - * - * Copyright 2013 Red Hat, Inc. - * - * 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.cli; - -import hudson.model.Computer; -import hudson.model.Messages; -import hudson.model.Slave; -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 static hudson.cli.CLICommandInvoker.Matcher.failedWith; -import static hudson.cli.CLICommandInvoker.Matcher.hasNoStandardOutput; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.junit.Assert.assertEquals; - -//TODO to be merged into base test after security release -public class UpdateNodeCommandSEC2021Test { - - private CLICommandInvoker command; - - @Rule public final JenkinsRule j = new JenkinsRule(); - - @Before public void setUp() { - - command = new CLICommandInvoker(j, new UpdateNodeCommand()); - } - - @Test - @Issue("SECURITY-2021") - public void updateNodeShouldFailForDotDot() throws Exception { - String okName = "MyNode"; - Slave node = j.createSlave(okName, null, null); - // currently , but doing so will be a bit more future-proof - String defaultDescription = node.getNodeDescription(); - - final CLICommandInvoker.Result result = command - .authorizedTo(Computer.CONFIGURE, Jenkins.READ) - .withStdin(UpdateNodeCommandSEC2021Test.class.getResourceAsStream("node_sec2021.xml")) - .invokeWithArgs(okName) - ; - - assertThat(result.stderr(), containsString(Messages.Hudson_UnsafeChar('/'))); - assertThat(result, hasNoStandardOutput()); - assertThat(result, failedWith(1)); - - assertEquals(okName, node.getNodeName()); - // ensure the other data were not saved - assertEquals(defaultDescription, node.getNodeDescription()); - } -} diff --git a/test/src/test/java/hudson/cli/UpdateNodeCommandTest.java b/test/src/test/java/hudson/cli/UpdateNodeCommandTest.java index efbee5715f..d136ee5fa9 100644 --- a/test/src/test/java/hudson/cli/UpdateNodeCommandTest.java +++ b/test/src/test/java/hudson/cli/UpdateNodeCommandTest.java @@ -31,8 +31,12 @@ import static org.hamcrest.Matchers.nullValue; import static hudson.cli.CLICommandInvoker.Matcher.failedWith; import static hudson.cli.CLICommandInvoker.Matcher.hasNoStandardOutput; import static hudson.cli.CLICommandInvoker.Matcher.succeededSilently; +import static org.junit.Assert.assertEquals; + import hudson.model.Computer; +import hudson.model.Messages; import hudson.model.Node; +import hudson.model.Slave; import jenkins.model.Jenkins; import org.junit.Before; @@ -111,4 +115,26 @@ public class UpdateNodeCommandTest { assertThat(result, hasNoStandardOutput()); } + @Test + @Issue("SECURITY-2021") + public void updateNodeShouldFailForDotDot() throws Exception { + String okName = "MyNode"; + Slave node = j.createSlave(okName, null, null); + // currently , but doing so will be a bit more future-proof + String defaultDescription = node.getNodeDescription(); + + final CLICommandInvoker.Result result = command + .authorizedTo(Computer.CONFIGURE, Jenkins.READ) + .withStdin(UpdateNodeCommandTest.class.getResourceAsStream("node_sec2021.xml")) + .invokeWithArgs(okName) + ; + + assertThat(result.stderr(), containsString(Messages.Hudson_UnsafeChar('/'))); + assertThat(result, hasNoStandardOutput()); + assertThat(result, failedWith(1)); + + assertEquals(okName, node.getNodeName()); + // ensure the other data were not saved + assertEquals(defaultDescription, node.getNodeDescription()); + } } diff --git a/test/src/test/java/hudson/markup/MarkupFormatterSecurity2153Test.java b/test/src/test/java/hudson/markup/MarkupFormatterSecurity2153Test.java deleted file mode 100644 index 5de9a32982..0000000000 --- a/test/src/test/java/hudson/markup/MarkupFormatterSecurity2153Test.java +++ /dev/null @@ -1,47 +0,0 @@ -package hudson.markup; - -import com.gargoylesoftware.htmlunit.HttpMethod; -import com.gargoylesoftware.htmlunit.Page; -import com.gargoylesoftware.htmlunit.WebRequest; -import com.gargoylesoftware.htmlunit.WebResponse; -import com.gargoylesoftware.htmlunit.html.HtmlPage; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.JenkinsRule; - -import java.net.URL; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; - -// TODO merge back into MarkupFormatterTest -public class MarkupFormatterSecurity2153Test { - - @Rule - public JenkinsRule j = new JenkinsRule(); - - @Test - public void security2153RequiresPOST() throws Exception { - final JenkinsRule.WebClient wc = j.createWebClient(); - wc.setThrowExceptionOnFailingStatusCode(false); - final HtmlPage htmlPage = wc.goTo("markupFormatter/previewDescription?text=lolwut"); - final WebResponse response = htmlPage.getWebResponse(); - assertEquals(405, response.getStatusCode()); - assertThat(response.getContentAsString(), containsString("This endpoint now requires that POST requests are sent")); - assertThat(response.getContentAsString(), not(containsString("lolwut"))); - } - - @Test - public void security2153SetsCSP() throws Exception { - final JenkinsRule.WebClient wc = j.createWebClient(); - final Page htmlPage = wc.getPage(wc.addCrumb(new WebRequest(new URL(j.jenkins.getRootUrl() + "/markupFormatter/previewDescription?text=lolwut"), HttpMethod.POST))); - final WebResponse response = htmlPage.getWebResponse(); - assertEquals(200, response.getStatusCode()); - assertThat(response.getContentAsString(), containsString("lolwut")); - assertThat(response.getResponseHeaderValue("Content-Security-Policy"), containsString("default-src 'none';")); - assertThat(response.getResponseHeaderValue("X-Content-Security-Policy"), containsString("default-src 'none';")); - assertThat(response.getResponseHeaderValue("X-WebKit-CSP"), containsString("default-src 'none';")); - } -} diff --git a/test/src/test/java/hudson/markup/MarkupFormatterTest.java b/test/src/test/java/hudson/markup/MarkupFormatterTest.java index daf03665a1..598985366b 100644 --- a/test/src/test/java/hudson/markup/MarkupFormatterTest.java +++ b/test/src/test/java/hudson/markup/MarkupFormatterTest.java @@ -23,18 +23,28 @@ */ package hudson.markup; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.Page; +import com.gargoylesoftware.htmlunit.WebRequest; +import com.gargoylesoftware.htmlunit.WebResponse; +import com.gargoylesoftware.htmlunit.html.HtmlPage; import hudson.security.AuthorizationStrategy.Unsecured; import hudson.security.HudsonPrivateSecurityRealm; 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.TestExtension; import org.kohsuke.stapler.DataBoundConstructor; import java.io.IOException; import java.io.Writer; +import java.net.URL; /** * @author Kohsuke Kawaguchi @@ -77,4 +87,28 @@ public class MarkupFormatterTest { assertEquals("", j.jenkins.getMarkupFormatter().translate(null)); } + @Test + @Issue("SECURITY-2153") + public void security2153RequiresPOST() throws Exception { + final JenkinsRule.WebClient wc = j.createWebClient(); + wc.setThrowExceptionOnFailingStatusCode(false); + final HtmlPage htmlPage = wc.goTo("markupFormatter/previewDescription?text=lolwut"); + final WebResponse response = htmlPage.getWebResponse(); + assertEquals(405, response.getStatusCode()); + assertThat(response.getContentAsString(), containsString("This endpoint now requires that POST requests are sent")); + assertThat(response.getContentAsString(), not(containsString("lolwut"))); + } + + @Test + @Issue("SECURITY-2153") + public void security2153SetsCSP() throws Exception { + final JenkinsRule.WebClient wc = j.createWebClient(); + final Page htmlPage = wc.getPage(wc.addCrumb(new WebRequest(new URL(j.jenkins.getRootUrl() + "/markupFormatter/previewDescription?text=lolwut"), HttpMethod.POST))); + final WebResponse response = htmlPage.getWebResponse(); + assertEquals(200, response.getStatusCode()); + assertThat(response.getContentAsString(), containsString("lolwut")); + assertThat(response.getResponseHeaderValue("Content-Security-Policy"), containsString("default-src 'none';")); + assertThat(response.getResponseHeaderValue("X-Content-Security-Policy"), containsString("default-src 'none';")); + assertThat(response.getResponseHeaderValue("X-WebKit-CSP"), containsString("default-src 'none';")); + } } diff --git a/test/src/test/java/hudson/model/ComputerConfigDotXmlSEC2021Test.java b/test/src/test/java/hudson/model/ComputerConfigDotXmlSEC2021Test.java deleted file mode 100644 index 28df421602..0000000000 --- a/test/src/test/java/hudson/model/ComputerConfigDotXmlSEC2021Test.java +++ /dev/null @@ -1,52 +0,0 @@ -package hudson.model; - -import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; -import com.gargoylesoftware.htmlunit.HttpMethod; -import com.gargoylesoftware.htmlunit.WebRequest; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.JenkinsRule; - -import java.io.File; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.not; -import static org.junit.Assert.fail; - -public class ComputerConfigDotXmlSEC2021Test { - - @Rule public final JenkinsRule rule = new JenkinsRule(); - - @Issue("SECURITY-2021") - @Test - public void nodeNameReferencesParentDir() throws Exception { - Computer computer = rule.createSlave("anything", null).toComputer(); - - JenkinsRule.WebClient wc = rule.createWebClient(); - WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%s/config.xml", computer.getUrl())), HttpMethod.POST); - req.setAdditionalHeader("Content-Type", "application/xml"); - req.setRequestBody(VALID_XML_BAD_NAME_XML); - - try { - wc.getPage(req); - fail("Should have returned failure."); - } catch (FailingHttpStatusCodeException e) { - assertThat(e.getStatusCode(), equalTo(400)); - } - File configDotXml = new File(rule.jenkins.getRootDir(), "config.xml"); - String configDotXmlContents = new String(Files.readAllBytes(configDotXml.toPath()), StandardCharsets.UTF_8); - - assertThat(configDotXmlContents, not(containsString("../"))); - } - - private static final String VALID_XML_BAD_NAME_XML = - "\n" + - " ../\n" + - ""; - -} diff --git a/test/src/test/java/hudson/model/ComputerConfigDotXmlTest.java b/test/src/test/java/hudson/model/ComputerConfigDotXmlTest.java index 489115df80..c24b26c80a 100644 --- a/test/src/test/java/hudson/model/ComputerConfigDotXmlTest.java +++ b/test/src/test/java/hudson/model/ComputerConfigDotXmlTest.java @@ -26,19 +26,28 @@ package hudson.model; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; + +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.WebRequest; import hudson.security.ACL; import hudson.security.AccessDeniedException3; import hudson.security.GlobalMatrixAuthorizationStrategy; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import javax.servlet.ReadListener; import javax.servlet.ServletInputStream; @@ -163,7 +172,32 @@ public class ComputerConfigDotXmlTest { assertFalse(computer.getMonitorData().isEmpty()); } + @Issue("SECURITY-2021") + @Test + public void nodeNameReferencesParentDir() throws Exception { + Computer computer = rule.createSlave("anything", null).toComputer(); + JenkinsRule.WebClient wc = rule.createWebClient(); + WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%s/config.xml", computer.getUrl())), HttpMethod.POST); + req.setAdditionalHeader("Content-Type", "application/xml"); + req.setRequestBody(VALID_XML_BAD_NAME_XML); + + try { + wc.getPage(req); + fail("Should have returned failure."); + } catch (FailingHttpStatusCodeException e) { + assertThat(e.getStatusCode(), equalTo(400)); + } + File configDotXml = new File(rule.jenkins.getRootDir(), "config.xml"); + String configDotXmlContents = new String(Files.readAllBytes(configDotXml.toPath()), StandardCharsets.UTF_8); + + assertThat(configDotXmlContents, not(containsString("../"))); + } + + private static final String VALID_XML_BAD_NAME_XML = + "\n" + + " ../\n" + + ""; private OutputStream captureOutput() throws IOException { diff --git a/test/src/test/java/hudson/model/ComputerSEC1923Test.java b/test/src/test/java/hudson/model/ComputerSEC1923Test.java deleted file mode 100644 index 3e1b64b740..0000000000 --- a/test/src/test/java/hudson/model/ComputerSEC1923Test.java +++ /dev/null @@ -1,81 +0,0 @@ -package hudson.model; - -import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; -import com.gargoylesoftware.htmlunit.HttpMethod; -import com.gargoylesoftware.htmlunit.WebRequest; -import hudson.ExtensionList; -import hudson.diagnosis.OldDataMonitor; -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.util.Map; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.fail; - -public class ComputerSEC1923Test { - - private static final String CONFIGURATOR = "configure_user"; - - @Rule - public JenkinsRule j = new JenkinsRule(); - - @Before - public void setupSecurity() { - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - MockAuthorizationStrategy mas = new MockAuthorizationStrategy(); - mas.grant(Computer.CONFIGURE, Computer.EXTENDED_READ, Jenkins.READ) - .everywhere() - .to(CONFIGURATOR); - j.jenkins.setAuthorizationStrategy(mas); - } - - @Issue("SECURITY-1923") - @Test - public void configDotXmlWithValidXmlAndBadField() throws Exception { - Computer computer = j.createSlave().toComputer(); - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.login(CONFIGURATOR); - WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%s/config.xml", computer.getUrl())), HttpMethod.POST); - req.setAdditionalHeader("Content-Type", "application/xml"); - req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); - - try { - wc.getPage(req); - fail("Should have returned failure."); - } catch (FailingHttpStatusCodeException e) { - // This really shouldn't return 500, but that's what it does now. - assertThat(e.getStatusCode(), equalTo(500)); - } - - OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); - Map data = odm.getData(); - - assertThat(data.size(), equalTo(0)); - - odm.doDiscard(null, null); - - User.AllUsers.scanAll(); - boolean createUser = false; - User badUser = User.getById("foo", createUser); - - assertNull("Should not have created user.", badUser); - } - - private static final String VALID_XML_BAD_FIELD_USER_XML = - "\n" + - " foo\n" + - " Foo User\n" + - " \n" + - "\n"; - - -} diff --git a/test/src/test/java/hudson/model/ComputerTest.java b/test/src/test/java/hudson/model/ComputerTest.java index e4280ff56b..735f28809f 100644 --- a/test/src/test/java/hudson/model/ComputerTest.java +++ b/test/src/test/java/hudson/model/ComputerTest.java @@ -26,19 +26,28 @@ package hudson.model; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.HttpMethod; import com.gargoylesoftware.htmlunit.Page; +import com.gargoylesoftware.htmlunit.WebRequest; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.xml.XmlPage; import java.io.File; import java.net.HttpURLConnection; import java.nio.charset.StandardCharsets; +import java.util.Map; +import hudson.ExtensionList; +import hudson.diagnosis.OldDataMonitor; import jenkins.model.Jenkins; import hudson.slaves.DumbSlave; import hudson.slaves.OfflineCause; @@ -49,6 +58,7 @@ import org.junit.experimental.categories.Category; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.JenkinsRule.WebClient; +import org.jvnet.hudson.test.MockAuthorizationStrategy; import org.jvnet.hudson.test.MockFolder; import org.jvnet.hudson.test.SmokeTest; import org.jvnet.hudson.test.recipes.LocalData; @@ -144,4 +154,52 @@ public class ComputerTest { assertThat(c.getTiedJobs(), containsInAnyOrder(p, p3)); } + @Issue("SECURITY-1923") + @Test + public void configDotXmlWithValidXmlAndBadField() throws Exception { + final String CONFIGURATOR = "configure_user"; + + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + MockAuthorizationStrategy mas = new MockAuthorizationStrategy(); + mas.grant(Computer.CONFIGURE, Computer.EXTENDED_READ, Jenkins.READ) + .everywhere() + .to(CONFIGURATOR); + j.jenkins.setAuthorizationStrategy(mas); + + Computer computer = j.createSlave().toComputer(); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.login(CONFIGURATOR); + WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%s/config.xml", computer.getUrl())), HttpMethod.POST); + req.setAdditionalHeader("Content-Type", "application/xml"); + req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); + + try { + wc.getPage(req); + fail("Should have returned failure."); + } catch (FailingHttpStatusCodeException e) { + // This really shouldn't return 500, but that's what it does now. + assertThat(e.getStatusCode(), equalTo(500)); + } + + OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); + Map data = odm.getData(); + + assertThat(data.size(), equalTo(0)); + + odm.doDiscard(null, null); + + User.AllUsers.scanAll(); + boolean createUser = false; + User badUser = User.getById("foo", createUser); + + assertNull("Should not have created user.", badUser); + } + + private static final String VALID_XML_BAD_FIELD_USER_XML = + "\n" + + " foo\n" + + " Foo User\n" + + " \n" + + "\n"; } diff --git a/test/src/test/java/hudson/model/FingerprintSEC2023Test.java b/test/src/test/java/hudson/model/FingerprintSEC2023Test.java deleted file mode 100644 index 304a9b232c..0000000000 --- a/test/src/test/java/hudson/model/FingerprintSEC2023Test.java +++ /dev/null @@ -1,262 +0,0 @@ -package hudson.model; - -import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; -import com.gargoylesoftware.htmlunit.Page; -import com.gargoylesoftware.htmlunit.WebRequest; -import edu.umd.cs.findbugs.annotations.CheckForNull; -import edu.umd.cs.findbugs.annotations.NonNull; -import hudson.Util; -import hudson.tasks.ArtifactArchiver; -import hudson.tasks.Fingerprinter; -import jenkins.fingerprints.FileFingerprintStorage; -import org.apache.commons.io.FileUtils; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.CreateFileBuilder; -import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.JenkinsRule; -import org.jvnet.hudson.test.LoggerRule; - -import java.io.File; -import java.net.URL; -import java.nio.charset.StandardCharsets; -import java.util.Map; -import java.util.logging.Level; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -public class FingerprintSEC2023Test { - - @Rule - public JenkinsRule rule = new JenkinsRule(); - - @Rule - public LoggerRule loggerRule = new LoggerRule(); - - @Test - public void checkNormalFingerprint() throws Exception { - FreeStyleProject project = rule.createFreeStyleProject(); - project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); - ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); - archiver.setFingerprint(true); - project.getPublishersList().add(archiver); - FreeStyleBuild build = rule.buildAndAssertSuccess(project); - - Fingerprint fp = getFingerprint(build, "test.txt"); - Fingerprint loadedFingerprint = Fingerprint.load(fp.getHashString()); - assertEquals(fp.getDisplayName(), loadedFingerprint.getDisplayName()); - } - - @Test - public void checkNormalFingerprintWithWebClient() throws Exception { - FreeStyleProject project = rule.createFreeStyleProject(); - project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); - ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); - archiver.setFingerprint(true); - project.getPublishersList().add(archiver); - FreeStyleBuild build = rule.buildAndAssertSuccess(project); - - Fingerprint fp = getFingerprint(build, "test.txt"); - - Page page = rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "fingerprint/" + fp.getHashString() + "/"))); - assertEquals(200, page.getWebResponse().getStatusCode()); - - // could also be reached using static// - Page page2 = rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "static/abc/fingerprint/" + fp.getHashString() + "/"))); - assertEquals(200, page2.getWebResponse().getStatusCode()); - } - - @Test - @Issue("SECURITY-2023") - public void checkArbitraryEmptyFileExistence() throws Exception { - FreeStyleProject project = rule.createFreeStyleProject(); - project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); - ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); - archiver.setFingerprint(true); - project.getPublishersList().add(archiver); - FreeStyleBuild build = rule.buildAndAssertSuccess(project); - - Fingerprint fp = getFingerprint(build, "test.txt"); - File targetFile = new File(rule.jenkins.getRootDir(), "../cf1.xml"); - Util.touch(targetFile); - // required as cf1.xml is outside the temporary folders created for the test - // and if the test is failing, it will not be deleted - targetFile.deleteOnExit(); - String first = fp.getHashString().substring(0,2); - String second = fp.getHashString().substring(2,4); - String id = first + second + "/../../" + first + "/" + second + "/../../../../cf1"; - Fingerprint fingerprint = Fingerprint.load(id); - assertNull(fingerprint); - assertTrue(targetFile.exists()); - } - - @Test - @Issue("SECURITY-2023") - public void checkArbitraryEmptyFileExistenceWithWebClient() throws Exception { - FreeStyleProject project = rule.createFreeStyleProject(); - project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); - ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); - archiver.setFingerprint(true); - project.getPublishersList().add(archiver); - FreeStyleBuild build = rule.buildAndAssertSuccess(project); - - Fingerprint fp = getFingerprint(build, "test.txt"); - File targetFile = new File(rule.jenkins.getRootDir(), "../cf2.xml"); - Util.touch(targetFile); - targetFile.deleteOnExit(); - String first = fp.getHashString().substring(0,2); - String second = fp.getHashString().substring(2,4); - rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "static/abc/fingerprint/" + first + second + "%2f..%2f..%2f" + first + "%2f" + second + "%2f..%2f..%2f..%2f..%2fcf2/"))); - assertTrue(targetFile.exists()); - } - - @Test - @Issue("SECURITY-2023") - public void checkArbitraryFileExistence() throws Exception { - FreeStyleProject project = rule.createFreeStyleProject(); - project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); - ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); - archiver.setFingerprint(true); - project.getPublishersList().add(archiver); - FreeStyleBuild build = rule.buildAndAssertSuccess(project); - - Fingerprint fp = getFingerprint(build, "test.txt"); - File sourceFile = new File(rule.jenkins.getRootDir(), "config.xml"); - File targetFile = new File(rule.jenkins.getRootDir(), "../cf3.xml"); - Util.copyFile(sourceFile, targetFile); - targetFile.deleteOnExit(); - String first = fp.getHashString().substring(0,2); - String second = fp.getHashString().substring(2,4); - String id = first + second + "/../../" + first + "/" + second + "/../../../../cf3"; - Fingerprint fingerprint = Fingerprint.load(id); - assertNull(fingerprint); - assertTrue(targetFile.exists()); - } - - @Test - @Issue("SECURITY-2023") - public void checkArbitraryFileExistenceWithWebClient() throws Exception { - loggerRule.record(FileFingerprintStorage.class, Level.WARNING) - .record(FileFingerprintStorage.class, Level.WARNING) - .capture(1000); - - FreeStyleProject project = rule.createFreeStyleProject(); - project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); - ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); - archiver.setFingerprint(true); - project.getPublishersList().add(archiver); - FreeStyleBuild build = rule.buildAndAssertSuccess(project); - - Fingerprint fp = getFingerprint(build, "test.txt"); - File sourceFile = new File(rule.jenkins.getRootDir(), "config.xml"); - File targetFile = new File(rule.jenkins.getRootDir(), "../cf4.xml"); - Util.copyFile(sourceFile, targetFile); - targetFile.deleteOnExit(); - String first = fp.getHashString().substring(0,2); - String second = fp.getHashString().substring(2,4); - - rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "static/abc/fingerprint/" + first + second + "%2f..%2f..%2f" + first + "%2f" + second + "%2f..%2f..%2f..%2f..%2fcf4/"))); - assertTrue(targetFile.exists()); - } - - @Test - @Issue("SECURITY-2023") - public void checkArbitraryFileNonexistence() throws Exception { - FreeStyleProject project = rule.createFreeStyleProject(); - project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); - ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); - archiver.setFingerprint(true); - project.getPublishersList().add(archiver); - FreeStyleBuild build = rule.buildAndAssertSuccess(project); - - Fingerprint fp = getFingerprint(build, "test.txt"); - String first = fp.getHashString().substring(0,2); - String second = fp.getHashString().substring(2,4); - String id = first + second + "/../../" + first + "/" + second + "/../../../../cf5"; - Fingerprint fingerprint = Fingerprint.load(id); - assertNull(fingerprint); - } - - @Test - @Issue("SECURITY-2023") - public void checkArbitraryFileNonexistenceWithWebClient() throws Exception { - FreeStyleProject project = rule.createFreeStyleProject(); - project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); - ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); - archiver.setFingerprint(true); - project.getPublishersList().add(archiver); - FreeStyleBuild build = rule.buildAndAssertSuccess(project); - - Fingerprint fp = getFingerprint(build, "test.txt"); - String first = fp.getHashString().substring(0,2); - String second = fp.getHashString().substring(2,4); - rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "static/abc/fingerprint/" + first + second + "%2f..%2f..%2f" + first + "%2f" + second + "%2f..%2f..%2f..%2f..%2fcf6/"))); - } - - @Test - @Issue("SECURITY-2023") - public void checkArbitraryFingerprintConfigFileExistenceWithWebClient() throws Exception { - FreeStyleProject project = rule.createFreeStyleProject(); - project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); - ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); - archiver.setFingerprint(true); - project.getPublishersList().add(archiver); - FreeStyleBuild build = rule.buildAndAssertSuccess(project); - - Fingerprint fp = getFingerprint(build, "test.txt"); - File targetFile = new File(rule.jenkins.getRootDir(), "../cf7.xml"); - FileUtils.writeStringToFile(targetFile, TEST_FINGERPRINT_CONFIG_FILE_CONTENT, StandardCharsets.UTF_8); - targetFile.deleteOnExit(); - - String first = fp.getHashString().substring(0,2); - String second = fp.getHashString().substring(2,4); - - Page page = null; - try { - // that file exists, so we need to ensure if it's returned, the content is not the expected one from the test data. - String partialUrl = "static/abc/fingerprint/" + first + second + "%2f..%2f..%2f" + first + "%2f" + second + "%2f..%2f..%2f..%2f..%2fcf7/"; - page = rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), partialUrl))); - } catch (FailingHttpStatusCodeException e) { - // expected refusal after the correction - assertEquals(500, e.getStatusCode()); - } - if (page != null) { - // content retrieval occurred before the correction, we have to check the content to ensure non-regression - String pageContent = page.getWebResponse().getContentAsString(); - assertThat(pageContent, not(containsString(TEST_FINGERPRINT_ID))); - } - assertTrue(targetFile.exists()); - } - - @NonNull - private Fingerprint getFingerprint(@CheckForNull Run run, @NonNull String filename) { - assertNotNull("Input run is null", run); - Fingerprinter.FingerprintAction action = run.getAction(Fingerprinter.FingerprintAction.class); - assertNotNull("Fingerprint action has not been created in " + run, action); - Map fingerprints = action.getFingerprints(); - final Fingerprint fp = fingerprints.get(filename); - assertNotNull("No reference to '" + filename + "' from the Fingerprint action", fp); - return fp; - } - - private static final String TEST_FINGERPRINT_ID = "0123456789abcdef0123456789abcdef"; - private static final String TEST_FINGERPRINT_CONFIG_FILE_CONTENT = "\n" + - "\n" + - " 2020-10-27 14:01:22.551 UTC\n" + - " \n" + - " test0\n" + - " 1\n" + - " \n" + - " "+TEST_FINGERPRINT_ID+"\n" + - " test.txt\n" + - " \n" + - " \n" + - " \n" + - ""; -} diff --git a/test/src/test/java/hudson/model/FingerprintTest.java b/test/src/test/java/hudson/model/FingerprintTest.java index eedcc9f8c2..9180bebcab 100644 --- a/test/src/test/java/hudson/model/FingerprintTest.java +++ b/test/src/test/java/hudson/model/FingerprintTest.java @@ -23,6 +23,9 @@ */ package hudson.model; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.Page; +import com.gargoylesoftware.htmlunit.WebRequest; import hudson.Util; import hudson.XmlFile; import hudson.security.ACL; @@ -36,17 +39,23 @@ import hudson.tasks.Fingerprinter; import java.io.File; import java.io.IOException; +import java.net.URL; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Hashtable; import java.util.Map; import java.util.Set; +import java.util.logging.Level; + import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import jenkins.fingerprints.FileFingerprintStorage; import jenkins.model.FingerprintFacet; import jenkins.model.Jenkins; +import org.apache.commons.io.FileUtils; +import org.hamcrest.Matchers; import org.junit.Rule; import org.junit.Test; import org.junit.Before; @@ -54,6 +63,7 @@ import org.junit.rules.TemporaryFolder; import org.jvnet.hudson.test.CreateFileBuilder; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.LoggerRule; import org.jvnet.hudson.test.MockFolder; import org.jvnet.hudson.test.SecuredMockFolder; import org.jvnet.hudson.test.WorkspaceCopyFileBuilder; @@ -65,6 +75,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.hamcrest.Matchers.notNullValue; @@ -85,7 +96,10 @@ public class FingerprintTest { @Rule public TemporaryFolder tmp = new TemporaryFolder(); - + + @Rule + public LoggerRule loggerRule = new LoggerRule(); + @Before public void setupRealm() { rule.jenkins.setSecurityRealm(rule.createDummySecurityRealm()); @@ -365,6 +379,201 @@ public class FingerprintTest { fingerprintLoaded = Fingerprint.load(id); assertThat(fingerprintLoaded, is(nullValue())); } + + @Test + public void checkNormalFingerprint() throws Exception { + FreeStyleProject project = rule.createFreeStyleProject(); + project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); + ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); + archiver.setFingerprint(true); + project.getPublishersList().add(archiver); + FreeStyleBuild build = rule.buildAndAssertSuccess(project); + + Fingerprint fp = getFingerprint(build, "test.txt"); + Fingerprint loadedFingerprint = Fingerprint.load(fp.getHashString()); + assertEquals(fp.getDisplayName(), loadedFingerprint.getDisplayName()); + } + + @Test + public void checkNormalFingerprintWithWebClient() throws Exception { + FreeStyleProject project = rule.createFreeStyleProject(); + project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); + ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); + archiver.setFingerprint(true); + project.getPublishersList().add(archiver); + FreeStyleBuild build = rule.buildAndAssertSuccess(project); + + Fingerprint fp = getFingerprint(build, "test.txt"); + + Page page = rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "fingerprint/" + fp.getHashString() + "/"))); + assertEquals(200, page.getWebResponse().getStatusCode()); + + // could also be reached using static// + Page page2 = rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "static/abc/fingerprint/" + fp.getHashString() + "/"))); + assertEquals(200, page2.getWebResponse().getStatusCode()); + } + + @Test + @Issue("SECURITY-2023") + public void checkArbitraryEmptyFileExistence() throws Exception { + FreeStyleProject project = rule.createFreeStyleProject(); + project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); + ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); + archiver.setFingerprint(true); + project.getPublishersList().add(archiver); + FreeStyleBuild build = rule.buildAndAssertSuccess(project); + + Fingerprint fp = getFingerprint(build, "test.txt"); + File targetFile = new File(rule.jenkins.getRootDir(), "../cf1.xml"); + Util.touch(targetFile); + // required as cf1.xml is outside the temporary folders created for the test + // and if the test is failing, it will not be deleted + targetFile.deleteOnExit(); + String first = fp.getHashString().substring(0,2); + String second = fp.getHashString().substring(2,4); + String id = first + second + "/../../" + first + "/" + second + "/../../../../cf1"; + Fingerprint fingerprint = Fingerprint.load(id); + assertNull(fingerprint); + assertTrue(targetFile.exists()); + } + + @Test + @Issue("SECURITY-2023") + public void checkArbitraryEmptyFileExistenceWithWebClient() throws Exception { + FreeStyleProject project = rule.createFreeStyleProject(); + project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); + ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); + archiver.setFingerprint(true); + project.getPublishersList().add(archiver); + FreeStyleBuild build = rule.buildAndAssertSuccess(project); + + Fingerprint fp = getFingerprint(build, "test.txt"); + File targetFile = new File(rule.jenkins.getRootDir(), "../cf2.xml"); + Util.touch(targetFile); + targetFile.deleteOnExit(); + String first = fp.getHashString().substring(0,2); + String second = fp.getHashString().substring(2,4); + rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "static/abc/fingerprint/" + first + second + "%2f..%2f..%2f" + first + "%2f" + second + "%2f..%2f..%2f..%2f..%2fcf2/"))); + assertTrue(targetFile.exists()); + } + + @Test + @Issue("SECURITY-2023") + public void checkArbitraryFileExistence() throws Exception { + FreeStyleProject project = rule.createFreeStyleProject(); + project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); + ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); + archiver.setFingerprint(true); + project.getPublishersList().add(archiver); + FreeStyleBuild build = rule.buildAndAssertSuccess(project); + + Fingerprint fp = getFingerprint(build, "test.txt"); + File sourceFile = new File(rule.jenkins.getRootDir(), "config.xml"); + File targetFile = new File(rule.jenkins.getRootDir(), "../cf3.xml"); + Util.copyFile(sourceFile, targetFile); + targetFile.deleteOnExit(); + String first = fp.getHashString().substring(0,2); + String second = fp.getHashString().substring(2,4); + String id = first + second + "/../../" + first + "/" + second + "/../../../../cf3"; + Fingerprint fingerprint = Fingerprint.load(id); + assertNull(fingerprint); + assertTrue(targetFile.exists()); + } + + @Test + @Issue("SECURITY-2023") + public void checkArbitraryFileExistenceWithWebClient() throws Exception { + loggerRule.record(FileFingerprintStorage.class, Level.WARNING) + .record(FileFingerprintStorage.class, Level.WARNING) + .capture(1000); + + FreeStyleProject project = rule.createFreeStyleProject(); + project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); + ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); + archiver.setFingerprint(true); + project.getPublishersList().add(archiver); + FreeStyleBuild build = rule.buildAndAssertSuccess(project); + + Fingerprint fp = getFingerprint(build, "test.txt"); + File sourceFile = new File(rule.jenkins.getRootDir(), "config.xml"); + File targetFile = new File(rule.jenkins.getRootDir(), "../cf4.xml"); + Util.copyFile(sourceFile, targetFile); + targetFile.deleteOnExit(); + String first = fp.getHashString().substring(0,2); + String second = fp.getHashString().substring(2,4); + + rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "static/abc/fingerprint/" + first + second + "%2f..%2f..%2f" + first + "%2f" + second + "%2f..%2f..%2f..%2f..%2fcf4/"))); + assertTrue(targetFile.exists()); + } + + @Test + @Issue("SECURITY-2023") + public void checkArbitraryFileNonexistence() throws Exception { + FreeStyleProject project = rule.createFreeStyleProject(); + project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); + ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); + archiver.setFingerprint(true); + project.getPublishersList().add(archiver); + FreeStyleBuild build = rule.buildAndAssertSuccess(project); + + Fingerprint fp = getFingerprint(build, "test.txt"); + String first = fp.getHashString().substring(0,2); + String second = fp.getHashString().substring(2,4); + String id = first + second + "/../../" + first + "/" + second + "/../../../../cf5"; + Fingerprint fingerprint = Fingerprint.load(id); + assertNull(fingerprint); + } + + @Test + @Issue("SECURITY-2023") + public void checkArbitraryFileNonexistenceWithWebClient() throws Exception { + FreeStyleProject project = rule.createFreeStyleProject(); + project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); + ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); + archiver.setFingerprint(true); + project.getPublishersList().add(archiver); + FreeStyleBuild build = rule.buildAndAssertSuccess(project); + + Fingerprint fp = getFingerprint(build, "test.txt"); + String first = fp.getHashString().substring(0,2); + String second = fp.getHashString().substring(2,4); + rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), "static/abc/fingerprint/" + first + second + "%2f..%2f..%2f" + first + "%2f" + second + "%2f..%2f..%2f..%2f..%2fcf6/"))); + } + + @Test + @Issue("SECURITY-2023") + public void checkArbitraryFingerprintConfigFileExistenceWithWebClient() throws Exception { + FreeStyleProject project = rule.createFreeStyleProject(); + project.getBuildersList().add(new CreateFileBuilder("test.txt", "Hello, world!")); + ArtifactArchiver archiver = new ArtifactArchiver("test.txt"); + archiver.setFingerprint(true); + project.getPublishersList().add(archiver); + FreeStyleBuild build = rule.buildAndAssertSuccess(project); + + Fingerprint fp = getFingerprint(build, "test.txt"); + File targetFile = new File(rule.jenkins.getRootDir(), "../cf7.xml"); + FileUtils.writeStringToFile(targetFile, TEST_FINGERPRINT_CONFIG_FILE_CONTENT, StandardCharsets.UTF_8); + targetFile.deleteOnExit(); + + String first = fp.getHashString().substring(0,2); + String second = fp.getHashString().substring(2,4); + + Page page = null; + try { + // that file exists, so we need to ensure if it's returned, the content is not the expected one from the test data. + String partialUrl = "static/abc/fingerprint/" + first + second + "%2f..%2f..%2f" + first + "%2f" + second + "%2f..%2f..%2f..%2f..%2fcf7/"; + page = rule.createWebClient().getPage(new WebRequest(new URL(rule.getURL(), partialUrl))); + } catch (FailingHttpStatusCodeException e) { + // expected refusal after the correction + assertEquals(500, e.getStatusCode()); + } + if (page != null) { + // content retrieval occurred before the correction, we have to check the content to ensure non-regression + String pageContent = page.getWebResponse().getContentAsString(); + assertThat(pageContent, Matchers.not(containsString(TEST_FINGERPRINT_ID))); + } + assertTrue(targetFile.exists()); + } @NonNull private Fingerprint getFingerprint(@CheckForNull Run run, @NonNull String filename) { @@ -443,4 +652,19 @@ public class FingerprintTest { } } } + + private static final String TEST_FINGERPRINT_ID = "0123456789abcdef0123456789abcdef"; + private static final String TEST_FINGERPRINT_CONFIG_FILE_CONTENT = "\n" + + "\n" + + " 2020-10-27 14:01:22.551 UTC\n" + + " \n" + + " test0\n" + + " 1\n" + + " \n" + + " "+TEST_FINGERPRINT_ID+"\n" + + " test.txt\n" + + " \n" + + " \n" + + " \n" + + ""; } diff --git a/test/src/test/java/hudson/model/FreeStyleProjectSEC1923Test.java b/test/src/test/java/hudson/model/FreeStyleProjectSEC1923Test.java deleted file mode 100644 index 78d74c5e88..0000000000 --- a/test/src/test/java/hudson/model/FreeStyleProjectSEC1923Test.java +++ /dev/null @@ -1,80 +0,0 @@ -package hudson.model; - -import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; -import com.gargoylesoftware.htmlunit.HttpMethod; -import com.gargoylesoftware.htmlunit.WebRequest; -import hudson.ExtensionList; -import hudson.diagnosis.OldDataMonitor; -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.util.Map; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.fail; - -public class FreeStyleProjectSEC1923Test { - - private static final String CONFIGURATOR = "configure_user"; - - @Rule - public JenkinsRule j = new JenkinsRule(); - - @Before - public void setupSecurity() { - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - MockAuthorizationStrategy mas = new MockAuthorizationStrategy(); - mas.grant(Item.CONFIGURE, Item.READ, Jenkins.READ) - .everywhere() - .to(CONFIGURATOR); - j.jenkins.setAuthorizationStrategy(mas); - } - - @Issue("SECURITY-1923") - @Test - public void configDotXmlWithValidXmlAndBadField() throws Exception { - FreeStyleProject project = j.createFreeStyleProject(); - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.login(CONFIGURATOR); - WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%s/config.xml", project.getUrl())), HttpMethod.POST); - req.setAdditionalHeader("Content-Type", "application/xml"); - req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); - - try { - wc.getPage(req); - fail("Should have returned failure."); - } catch (FailingHttpStatusCodeException e) { - // This really shouldn't return 500, but that's what it does now. - assertThat(e.getStatusCode(), equalTo(500)); - } - - OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); - Map data = odm.getData(); - - assertThat(data.size(), equalTo(0)); - - odm.doDiscard(null, null); - - User.AllUsers.scanAll(); - boolean createUser = false; - User badUser = User.getById("foo", createUser); - - assertNull("Should not have created user.", badUser); - } - - private static final String VALID_XML_BAD_FIELD_USER_XML = - "\n" + - " foo\n" + - " Foo User\n" + - " \n" + - "\n"; - -} diff --git a/test/src/test/java/hudson/model/FreeStyleProjectTest.java b/test/src/test/java/hudson/model/FreeStyleProjectTest.java index ebd46a53b4..38090af4c5 100644 --- a/test/src/test/java/hudson/model/FreeStyleProjectTest.java +++ b/test/src/test/java/hudson/model/FreeStyleProjectTest.java @@ -23,18 +23,27 @@ */ package hudson.model; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotSame; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.WebRequest; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlPage; +import hudson.ExtensionList; +import hudson.diagnosis.OldDataMonitor; import hudson.tasks.Builder; import hudson.tasks.Shell; import java.io.ByteArrayInputStream; +import jenkins.model.Jenkins; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -42,10 +51,12 @@ import org.jvnet.hudson.test.FailureBuilder; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.JenkinsRule.WebClient; +import org.jvnet.hudson.test.MockAuthorizationStrategy; import org.jvnet.hudson.test.SmokeTest; import java.util.List; import java.io.File; +import java.util.Map; /** * @author Kohsuke Kawaguchi @@ -152,4 +163,53 @@ public class FreeStyleProjectTest { assertThat(String.format("Pattern %s score", Integer.toBinaryString(i)), health.getScore(), is(100*(5-expectedFails)/5)); } } + + @Issue("SECURITY-1923") + @Test + public void configDotXmlWithValidXmlAndBadField() throws Exception { + final String CONFIGURATOR = "configure_user"; + + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + MockAuthorizationStrategy mas = new MockAuthorizationStrategy(); + mas.grant(Item.CONFIGURE, Item.READ, Jenkins.READ) + .everywhere() + .to(CONFIGURATOR); + j.jenkins.setAuthorizationStrategy(mas); + + FreeStyleProject project = j.createFreeStyleProject(); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.login(CONFIGURATOR); + WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%s/config.xml", project.getUrl())), HttpMethod.POST); + req.setAdditionalHeader("Content-Type", "application/xml"); + req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); + + try { + wc.getPage(req); + fail("Should have returned failure."); + } catch (FailingHttpStatusCodeException e) { + // This really shouldn't return 500, but that's what it does now. + assertThat(e.getStatusCode(), equalTo(500)); + } + + OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); + Map data = odm.getData(); + + assertThat(data.size(), equalTo(0)); + + odm.doDiscard(null, null); + + User.AllUsers.scanAll(); + boolean createUser = false; + User badUser = User.getById("foo", createUser); + + assertNull("Should not have created user.", badUser); + } + + private static final String VALID_XML_BAD_FIELD_USER_XML = + "\n" + + " foo\n" + + " Foo User\n" + + " \n" + + "\n"; } diff --git a/test/src/test/java/hudson/model/ItemGroupMixInSEC1923Test.java b/test/src/test/java/hudson/model/ItemGroupMixInSEC1923Test.java deleted file mode 100644 index 05f1d0f109..0000000000 --- a/test/src/test/java/hudson/model/ItemGroupMixInSEC1923Test.java +++ /dev/null @@ -1,81 +0,0 @@ -package hudson.model; - - -import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; -import com.gargoylesoftware.htmlunit.HttpMethod; -import com.gargoylesoftware.htmlunit.WebRequest; -import hudson.ExtensionList; -import hudson.diagnosis.OldDataMonitor; -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.net.URL; -import java.util.Map; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.fail; - -public class ItemGroupMixInSEC1923Test { - - private static final String CREATOR = "create_user"; - - @Rule - public JenkinsRule j = new JenkinsRule(); - - @Before - public void setupSecurity() { - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - MockAuthorizationStrategy mas = new MockAuthorizationStrategy(); - mas.grant(Item.CREATE, Item.CONFIGURE, Item.READ, Jenkins.READ) - .everywhere() - .to(CREATOR); - j.jenkins.setAuthorizationStrategy(mas); - } - - @Issue("SECURITY-1923") - @Test - public void doCreateItemWithValidXmlAndBadField() throws Exception { - JenkinsRule.WebClient wc = j.createWebClient(); - wc.login(CREATOR); - WebRequest req = new WebRequest(wc.createCrumbedUrl("createItem?name=testProject"), HttpMethod.POST); - req.setAdditionalHeader("Content-Type", "application/xml"); - req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); - - try { - wc.getPage(req); - fail("Should have returned failure."); - } catch (FailingHttpStatusCodeException e) { - // This really shouldn't return 500, but that's what it does now. - assertThat(e.getStatusCode(), equalTo(500)); - } - - OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); - Map data = odm.getData(); - - assertThat(data.size(), equalTo(0)); - - odm.doDiscard(null, null); - - User.AllUsers.scanAll(); - boolean createUser = false; - User badUser = User.getById("foo", createUser); - - assertNull("Should not have created user.", badUser); - } - - private static final String VALID_XML_BAD_FIELD_USER_XML = - "\n" + - " foo\n" + - " Foo User\n" + - " \n" + - "\n"; -} diff --git a/test/src/test/java/hudson/model/ItemGroupMixInTest.java b/test/src/test/java/hudson/model/ItemGroupMixInTest.java index 3e44222c7b..60cabb1111 100644 --- a/test/src/test/java/hudson/model/ItemGroupMixInTest.java +++ b/test/src/test/java/hudson/model/ItemGroupMixInTest.java @@ -24,7 +24,12 @@ package hudson.model; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.WebRequest; import hudson.Extension; +import hudson.ExtensionList; +import hudson.diagnosis.OldDataMonitor; import hudson.tasks.BuildStepDescriptor; import hudson.tasks.BuildStepMonitor; import hudson.tasks.BuildTrigger; @@ -33,11 +38,13 @@ import hudson.tasks.BuildWrapperDescriptor; import hudson.tasks.Builder; import hudson.tasks.Publisher; import hudson.triggers.Trigger; +import jenkins.model.Jenkins; import org.apache.commons.io.FileUtils; import java.io.ByteArrayInputStream; import java.io.IOException; +import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.hamcrest.MatcherAssert.assertThat; @@ -46,6 +53,7 @@ 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.MockFolder; import org.jvnet.hudson.test.TestExtension; import org.jvnet.hudson.test.recipes.LocalData; @@ -53,10 +61,12 @@ import org.jvnet.hudson.test.recipes.LocalData; import java.io.File; import java.util.Collection; import java.util.List; +import java.util.Map; import static org.hamcrest.core.StringContains.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; +import static org.junit.Assert.fail; public class ItemGroupMixInTest { @@ -259,4 +269,51 @@ public class ItemGroupMixInTest { }); assertEquals(exception.getMessage(), Messages.Hudson_UnsafeChar("@")); } + + @Issue("SECURITY-1923") + @Test + public void doCreateItemWithValidXmlAndBadField() throws Exception { + final String CREATOR = "create_user"; + + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + MockAuthorizationStrategy mas = new MockAuthorizationStrategy(); + mas.grant(Item.CREATE, Item.CONFIGURE, Item.READ, Jenkins.READ) + .everywhere() + .to(CREATOR); + r.jenkins.setAuthorizationStrategy(mas); + + JenkinsRule.WebClient wc = r.createWebClient(); + wc.login(CREATOR); + WebRequest req = new WebRequest(wc.createCrumbedUrl("createItem?name=testProject"), HttpMethod.POST); + req.setAdditionalHeader("Content-Type", "application/xml"); + req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); + + try { + wc.getPage(req); + fail("Should have returned failure."); + } catch (FailingHttpStatusCodeException e) { + // This really shouldn't return 500, but that's what it does now. + assertThat(e.getStatusCode(), equalTo(500)); + } + + OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); + Map data = odm.getData(); + + assertThat(data.size(), equalTo(0)); + + odm.doDiscard(null, null); + + User.AllUsers.scanAll(); + boolean createUser = false; + User badUser = User.getById("foo", createUser); + + assertNull("Should not have created user.", badUser); + } + + private static final String VALID_XML_BAD_FIELD_USER_XML = + "\n" + + " foo\n" + + " Foo User\n" + + " \n" + + "\n"; } diff --git a/test/src/test/java/hudson/model/ViewSEC1923Test.java b/test/src/test/java/hudson/model/ViewSEC1923Test.java deleted file mode 100644 index fec5ff43ac..0000000000 --- a/test/src/test/java/hudson/model/ViewSEC1923Test.java +++ /dev/null @@ -1,187 +0,0 @@ -package hudson.model; - -import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; -import com.gargoylesoftware.htmlunit.HttpMethod; -import com.gargoylesoftware.htmlunit.WebRequest; -import hudson.ExtensionList; -import hudson.diagnosis.OldDataMonitor; -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.util.Map; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.fail; - -public class ViewSEC1923Test { - - private static final String CREATE_VIEW = "create_view"; - private static final String CONFIGURATOR = "configure_user"; - - @Rule - public JenkinsRule j = new JenkinsRule(); - - @Before - public void setupSecurity() { - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - MockAuthorizationStrategy mas = new MockAuthorizationStrategy(); - mas.grant(View.CREATE, View.READ, Jenkins.READ) - .everywhere() - .to(CREATE_VIEW); - mas.grant(View.CONFIGURE, View.READ, Jenkins.READ) - .everywhere() - .to(CONFIGURATOR); - j.jenkins.setAuthorizationStrategy(mas); - } - - @Test - @Issue("SECURITY-1923") - public void simplifiedOriginalDescription() throws Exception { - /* This is a simplified version of the original report in SECURITY-1923. - The XML is broken, because the root element doesn't have a matching end. - The last line is almost a matching end, but it lacks the slash character. - Instead that line gets interpreted as another contained element, one that - doesn't actually exist on the class. This causes it to get logged by the - old data monitor. */ - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.login(CREATE_VIEW); - - /* The view to create has to be nonexistent, otherwise a different code path is followed - and the vulnerability doesn't manifest. */ - WebRequest req = new WebRequest(wc.createCrumbedUrl("createView?name=nonexistent"), HttpMethod.POST); - req.setAdditionalHeader("Content-Type", "application/xml"); - req.setRequestBody(ORIGINAL_BAD_USER_XML); - - try { - wc.getPage(req); - fail("Should have returned failure."); - } catch (FailingHttpStatusCodeException e) { - // This really shouldn't return 500, but that's what it does now. - assertThat(e.getStatusCode(), equalTo(500)); - - // This should have a different message, but this is the current behavior demonstrating the problem. - assertThat(e.getResponse().getContentAsString(), containsString("A problem occurred while processing the request.")); - } - - OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); - Map data = odm.getData(); - - assertThat(data.size(), equalTo(0)); - - odm.doDiscard(null, null); - - View view = j.getInstance().getView("nonexistent"); - - // The view should still be nonexistent, as we gave it a user and not a view. - assertNull("Should not have created view.", view); - - User.AllUsers.scanAll(); - boolean createUser = false; - User badUser = User.getById("foo", createUser); - - assertNull("Should not have created user.", badUser); - } - - @Test - @Issue("SECURITY-1923") - public void simplifiedWithValidXmlAndBadField() throws Exception { - /* This is the same thing as the original report, except it uses valid XML. - It just adds in additional invalid field, which gets picked up by the old data monitor. - Way too much duplicated code here, but this is just for demonstration. */ - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.login(CREATE_VIEW); - - /* The view to create has to be nonexistent, otherwise a different code path is followed - and the vulnerability doesn't manifest. */ - WebRequest req = new WebRequest(wc.createCrumbedUrl("createView?name=nonexistent"), HttpMethod.POST); - req.setAdditionalHeader("Content-Type", "application/xml"); - req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); - - try { - wc.getPage(req); - fail("Should have returned failure."); - } catch (FailingHttpStatusCodeException e) { - // This really shouldn't return 500, but that's what it does now. - assertThat(e.getStatusCode(), equalTo(500)); - - // This should have a different message, but this is the current behavior demonstrating the problem. - assertThat(e.getResponse().getContentAsString(), containsString("A problem occurred while processing the request.")); - } - - OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); - Map data = odm.getData(); - - assertThat(data.size(), equalTo(0)); - - odm.doDiscard(null, null); - - View view = j.getInstance().getView("nonexistent"); - - // The view should still be nonexistent, as we gave it a user and not a view. - assertNull("Should not have created view.", view); - - User.AllUsers.scanAll(); - boolean createUser = false; - User badUser = User.getById("foo", createUser); - - assertNull("Should not have created user.", badUser); - } - - @Test - @Issue("SECURITY-1923") - public void configDotXmlWithValidXmlAndBadField() throws Exception { - ListView view = new ListView("view1", j.jenkins); - j.jenkins.addView(view); - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.login(CONFIGURATOR); - WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%s/config.xml", view.getUrl())), HttpMethod.POST); - req.setAdditionalHeader("Content-Type", "application/xml"); - req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); - - try { - wc.getPage(req); - fail("Should have returned failure."); - } catch (FailingHttpStatusCodeException e) { - // This really shouldn't return 500, but that's what it does now. - assertThat(e.getStatusCode(), equalTo(500)); - } - - OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); - Map data = odm.getData(); - - assertThat(data.size(), equalTo(0)); - - odm.doDiscard(null, null); - - User.AllUsers.scanAll(); - boolean createUser = false; - User badUser = User.getById("foo", createUser); - - assertNull("Should not have created user.", badUser); - } - - private static final String VALID_XML_BAD_FIELD_USER_XML = - "\n" + - " foo\n" + - " Foo User\n" + - " \n" + - "\n"; - - private static final String ORIGINAL_BAD_USER_XML = - "\n" + - " foo\n" + - " Foo User\n" + - "\n"; - -} diff --git a/test/src/test/java/hudson/model/ViewSEC2171Test.java b/test/src/test/java/hudson/model/ViewSEC2171Test.java deleted file mode 100644 index 98197de103..0000000000 --- a/test/src/test/java/hudson/model/ViewSEC2171Test.java +++ /dev/null @@ -1,214 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Tom Huybrechts - * - * 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.model; - -import com.gargoylesoftware.htmlunit.html.HtmlPage; -import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLElement; -import edu.umd.cs.findbugs.annotations.CheckForNull; -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.TestExtension; - -import java.util.Arrays; -import java.util.List; - -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.hasItem; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.CoreMatchers.not; -import static org.hamcrest.CoreMatchers.nullValue; -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; - -/** - * @author Kohsuke Kawaguchi - */ -//TODO to be merged back into ViewTest after the security release -public class ViewSEC2171Test { - - @Rule public JenkinsRule j = new JenkinsRule(); - - @Test - @Issue("SECURITY-2171") - public void newJob_xssPreventedInId() throws Exception { - CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); - customizableTLID.customId = "regularclass\" onclick=alert(123) other=\""; - customizableTLID.customDisplayName = "DN-xss-id"; - - JenkinsRule.WebClient wc = j.createWebClient(); - - HtmlPage page = wc.goTo("view/all/newJob"); - - Object result = page.executeJavaScript("Array.from(document.querySelectorAll('.label')).filter(el => el.innerText.indexOf('" + customizableTLID.customDisplayName + "') !== -1)[0].parentElement.parentElement").getJavaScriptResult(); - assertThat(result, instanceOf(HTMLElement.class)); - HTMLElement resultElement = (HTMLElement) result; - assertThat(resultElement.getAttribute("onclick", null), nullValue()); - } - - @Test - @Issue("SECURITY-2171") - public void newJob_xssPreventedInDisplayName() throws Exception { - CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); - customizableTLID.customId = "xss-dn"; - customizableTLID.customDisplayName = "DN "; - - JenkinsRule.WebClient wc = j.createWebClient(); - - HtmlPage page = wc.goTo("view/all/newJob"); - - Object result = page.executeJavaScript("document.querySelector('.xss-dn .label').innerHTML").getJavaScriptResult(); - assertThat(result, instanceOf(String.class)); - String resultString = (String) result; - assertThat(resultString, not(containsString("<"))); - } - - @Test - public void newJob_descriptionSupportsHtml() throws Exception { - CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); - customizableTLID.customId = "html-desc"; - customizableTLID.customDescription = "Super looong description"; - - JenkinsRule.WebClient wc = j.createWebClient(); - - HtmlPage page = wc.goTo("view/all/newJob"); - - Object result = page.executeJavaScript("document.querySelector('.html-desc .desc strong')").getJavaScriptResult(); - assertThat(result, instanceOf(HTMLElement.class)); - assertThat(((HTMLElement) result).getTagName(), is("STRONG")); - } - - @Test - @Issue("SECURITY-2171") - public void newJob_xssPreventedInGetIconFilePathPattern() throws Exception { - CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); - customizableTLID.customId = "xss-ifpp"; - customizableTLID.customIconClassName = null; - customizableTLID.customIconFilePathPattern = "\"> resultClassNamesList = Arrays.asList(resultClassNamesString.split(" ")); - assertThat(resultClassNamesList, hasItem("icon-xlg")); - assertThat(resultClassNamesList, hasItem("icon-freestyle-project")); - - Object resultSrc = page.executeJavaScript("document.querySelector('.hudson_model_FreeStyleProject .icon img').src").getJavaScriptResult(); - assertThat(resultSrc, instanceOf(String.class)); - String resultSrcString = (String) resultSrc; - assertThat(resultSrcString, containsString("48x48")); - assertThat(resultSrcString, containsString("freestyleproject.png")); - } - - @Test - public void newJob_twoLetterIcon() throws Exception { - CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); - customizableTLID.customId = "two-letters-desc"; - customizableTLID.customDisplayName = "Two words"; - customizableTLID.customIconClassName = null; - customizableTLID.customIconFilePathPattern = null; - - JenkinsRule.WebClient wc = j.createWebClient(); - - HtmlPage page = wc.goTo("view/all/newJob"); - - Object result = page.executeJavaScript("document.querySelector('." + customizableTLID.customId + " .default-icon')").getJavaScriptResult(); - assertThat(result, instanceOf(HTMLElement.class)); - HTMLElement resultHtml = (HTMLElement) result; - HTMLElement spanA = (HTMLElement) resultHtml.getFirstElementChild(); - HTMLElement spanB = (HTMLElement) resultHtml.getLastElementChild(); - assertThat(spanA.getClassName_js(), is("a")); - assertThat(spanA.getInnerText(), is("T")); - assertThat(spanB.getClassName_js(), is("b")); - assertThat(spanB.getInnerText(), is("w")); - } - - @TestExtension - public static class CustomizableTLID extends TopLevelItemDescriptor { - - public String customId = "ID-not-yet-defined"; - public String customDisplayName = "DisplayName-not-yet-defined"; - public String customDescription = "Description-not-yet-defined"; - public String customIconFilePathPattern = "IconFilePathPattern-not-yet-defined"; - public String customIconClassName = "IconClassName-not-yet-defined"; - - public CustomizableTLID() { - super(FreeStyleProject.class); - } - - @Override - public String getId() { - return customId; - } - - @Override - public String getDisplayName() { - return customDisplayName; - } - - @Override - public String getDescription() { - return customDescription; - } - - @Override - public @CheckForNull String getIconFilePathPattern() { - return customIconFilePathPattern; - } - - @Override - public String getIconClassName() { - return customIconClassName; - } - - @Override - public TopLevelItem newInstance(ItemGroup parent, String name) { - throw new UnsupportedOperationException(); - } - - } -} diff --git a/test/src/test/java/hudson/model/ViewTest.java b/test/src/test/java/hudson/model/ViewTest.java index 54506d2275..3f948d36ac 100644 --- a/test/src/test/java/hudson/model/ViewTest.java +++ b/test/src/test/java/hudson/model/ViewTest.java @@ -24,9 +24,14 @@ package hudson.model; import com.cloudbees.hudson.plugins.folder.Folder; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.WebRequest; import com.gargoylesoftware.htmlunit.html.DomNodeUtil; +import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLElement; +import edu.umd.cs.findbugs.annotations.CheckForNull; +import hudson.ExtensionList; +import hudson.diagnosis.OldDataMonitor; import hudson.views.ViewsTabBar; import jenkins.model.Jenkins; import org.jenkins.ui.icon.Icon; @@ -60,6 +65,7 @@ import hudson.util.FormValidation; import hudson.util.HudsonIsLoading; import java.io.File; import java.io.IOException; +import java.util.Arrays; import java.util.Collection; import java.net.HttpURLConnection; import java.util.Collections; @@ -72,7 +78,13 @@ import jenkins.model.ProjectNamingStrategy; import jenkins.security.NotReallyRoleSensitiveCallable; import static java.util.Arrays.asList; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.Matchers.*; +import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -102,6 +114,9 @@ import org.kohsuke.stapler.DataBoundConstructor; */ public class ViewTest { + private static final String CREATE_VIEW = "create_view"; + private static final String CONFIGURATOR = "configure_user"; + @Rule public JenkinsRule j = new JenkinsRule(); @Rule public LoggerRule logging = new LoggerRule(); @@ -654,6 +669,327 @@ public class ViewTest { return given(mock(View.class).getViewName()).willReturn(viewName).getMock(); } + public void prepareSec1923() { + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + MockAuthorizationStrategy mas = new MockAuthorizationStrategy(); + mas.grant(View.CREATE, View.READ, Jenkins.READ) + .everywhere() + .to(CREATE_VIEW); + mas.grant(View.CONFIGURE, View.READ, Jenkins.READ) + .everywhere() + .to(CONFIGURATOR); + j.jenkins.setAuthorizationStrategy(mas); + } + + @Test + @Issue("SECURITY-1923") + public void simplifiedOriginalDescription() throws Exception { + this.prepareSec1923(); + + /* This is a simplified version of the original report in SECURITY-1923. + The XML is broken, because the root element doesn't have a matching end. + The last line is almost a matching end, but it lacks the slash character. + Instead that line gets interpreted as another contained element, one that + doesn't actually exist on the class. This causes it to get logged by the + old data monitor. */ + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.login(CREATE_VIEW); + + /* The view to create has to be nonexistent, otherwise a different code path is followed + and the vulnerability doesn't manifest. */ + WebRequest req = new WebRequest(wc.createCrumbedUrl("createView?name=nonexistent"), HttpMethod.POST); + req.setAdditionalHeader("Content-Type", "application/xml"); + req.setRequestBody(ORIGINAL_BAD_USER_XML); + + try { + wc.getPage(req); + fail("Should have returned failure."); + } catch (FailingHttpStatusCodeException e) { + // This really shouldn't return 500, but that's what it does now. + assertThat(e.getStatusCode(), equalTo(500)); + + // This should have a different message, but this is the current behavior demonstrating the problem. + assertThat(e.getResponse().getContentAsString(), containsString("A problem occurred while processing the request.")); + } + + OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); + Map data = odm.getData(); + + assertThat(data.size(), equalTo(0)); + + odm.doDiscard(null, null); + + View view = j.getInstance().getView("nonexistent"); + + // The view should still be nonexistent, as we gave it a user and not a view. + assertNull("Should not have created view.", view); + + User.AllUsers.scanAll(); + boolean createUser = false; + User badUser = User.getById("foo", createUser); + + assertNull("Should not have created user.", badUser); + } + + @Test + @Issue("SECURITY-1923") + public void simplifiedWithValidXmlAndBadField() throws Exception { + this.prepareSec1923(); + + /* This is the same thing as the original report, except it uses valid XML. + It just adds in additional invalid field, which gets picked up by the old data monitor. + Way too much duplicated code here, but this is just for demonstration. */ + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.login(CREATE_VIEW); + + /* The view to create has to be nonexistent, otherwise a different code path is followed + and the vulnerability doesn't manifest. */ + WebRequest req = new WebRequest(wc.createCrumbedUrl("createView?name=nonexistent"), HttpMethod.POST); + req.setAdditionalHeader("Content-Type", "application/xml"); + req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); + + try { + wc.getPage(req); + fail("Should have returned failure."); + } catch (FailingHttpStatusCodeException e) { + // This really shouldn't return 500, but that's what it does now. + assertThat(e.getStatusCode(), equalTo(500)); + + // This should have a different message, but this is the current behavior demonstrating the problem. + assertThat(e.getResponse().getContentAsString(), containsString("A problem occurred while processing the request.")); + } + + OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); + Map data = odm.getData(); + + assertThat(data.size(), equalTo(0)); + + odm.doDiscard(null, null); + + View view = j.getInstance().getView("nonexistent"); + + // The view should still be nonexistent, as we gave it a user and not a view. + assertNull("Should not have created view.", view); + + User.AllUsers.scanAll(); + boolean createUser = false; + User badUser = User.getById("foo", createUser); + + assertNull("Should not have created user.", badUser); + } + + @Test + @Issue("SECURITY-1923") + public void configDotXmlWithValidXmlAndBadField() throws Exception { + this.prepareSec1923(); + + ListView view = new ListView("view1", j.jenkins); + j.jenkins.addView(view); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.login(CONFIGURATOR); + WebRequest req = new WebRequest(wc.createCrumbedUrl(String.format("%s/config.xml", view.getUrl())), HttpMethod.POST); + req.setAdditionalHeader("Content-Type", "application/xml"); + req.setRequestBody(VALID_XML_BAD_FIELD_USER_XML); + + try { + wc.getPage(req); + fail("Should have returned failure."); + } catch (FailingHttpStatusCodeException e) { + // This really shouldn't return 500, but that's what it does now. + assertThat(e.getStatusCode(), equalTo(500)); + } + + OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class); + Map data = odm.getData(); + + assertThat(data.size(), equalTo(0)); + + odm.doDiscard(null, null); + + User.AllUsers.scanAll(); + boolean createUser = false; + User badUser = User.getById("foo", createUser); + + assertNull("Should not have created user.", badUser); + } + + private static final String VALID_XML_BAD_FIELD_USER_XML = + "\n" + + " foo\n" + + " Foo User\n" + + " \n" + + "\n"; + + private static final String ORIGINAL_BAD_USER_XML = + "\n" + + " foo\n" + + " Foo User\n" + + "\n"; + + + @Test + @Issue("SECURITY-2171") + public void newJob_xssPreventedInId() throws Exception { + CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); + customizableTLID.customId = "regularclass\" onclick=alert(123) other=\""; + customizableTLID.customDisplayName = "DN-xss-id"; + + JenkinsRule.WebClient wc = j.createWebClient(); + + HtmlPage page = wc.goTo("view/all/newJob"); + + Object result = page.executeJavaScript("Array.from(document.querySelectorAll('.label')).filter(el => el.innerText.indexOf('" + customizableTLID.customDisplayName + "') !== -1)[0].parentElement.parentElement").getJavaScriptResult(); + assertThat(result, instanceOf(HTMLElement.class)); + HTMLElement resultElement = (HTMLElement) result; + assertThat(resultElement.getAttribute("onclick", null), nullValue()); + } + + @Test + @Issue("SECURITY-2171") + public void newJob_xssPreventedInDisplayName() throws Exception { + CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); + customizableTLID.customId = "xss-dn"; + customizableTLID.customDisplayName = "DN "; + + JenkinsRule.WebClient wc = j.createWebClient(); + + HtmlPage page = wc.goTo("view/all/newJob"); + + Object result = page.executeJavaScript("document.querySelector('.xss-dn .label').innerHTML").getJavaScriptResult(); + assertThat(result, instanceOf(String.class)); + String resultString = (String) result; + assertThat(resultString, not(containsString("<"))); + } + + @Test + public void newJob_descriptionSupportsHtml() throws Exception { + CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); + customizableTLID.customId = "html-desc"; + customizableTLID.customDescription = "Super looong description"; + + JenkinsRule.WebClient wc = j.createWebClient(); + + HtmlPage page = wc.goTo("view/all/newJob"); + + Object result = page.executeJavaScript("document.querySelector('.html-desc .desc strong')").getJavaScriptResult(); + assertThat(result, instanceOf(HTMLElement.class)); + assertThat(((HTMLElement) result).getTagName(), is("STRONG")); + } + + @Test + @Issue("SECURITY-2171") + public void newJob_xssPreventedInGetIconFilePathPattern() throws Exception { + CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); + customizableTLID.customId = "xss-ifpp"; + customizableTLID.customIconClassName = null; + customizableTLID.customIconFilePathPattern = "\"> resultClassNamesList = Arrays.asList(resultClassNamesString.split(" ")); + assertThat(resultClassNamesList, hasItem("icon-xlg")); + assertThat(resultClassNamesList, hasItem("icon-freestyle-project")); + + Object resultSrc = page.executeJavaScript("document.querySelector('.hudson_model_FreeStyleProject .icon img').src").getJavaScriptResult(); + assertThat(resultSrc, instanceOf(String.class)); + String resultSrcString = (String) resultSrc; + assertThat(resultSrcString, containsString("48x48")); + assertThat(resultSrcString, containsString("freestyleproject.png")); + } + + @Test + public void newJob_twoLetterIcon() throws Exception { + CustomizableTLID customizableTLID = j.jenkins.getExtensionList(TopLevelItemDescriptor.class).get(CustomizableTLID.class); + customizableTLID.customId = "two-letters-desc"; + customizableTLID.customDisplayName = "Two words"; + customizableTLID.customIconClassName = null; + customizableTLID.customIconFilePathPattern = null; + + JenkinsRule.WebClient wc = j.createWebClient(); + + HtmlPage page = wc.goTo("view/all/newJob"); + + Object result = page.executeJavaScript("document.querySelector('." + customizableTLID.customId + " .default-icon')").getJavaScriptResult(); + assertThat(result, instanceOf(HTMLElement.class)); + HTMLElement resultHtml = (HTMLElement) result; + HTMLElement spanA = (HTMLElement) resultHtml.getFirstElementChild(); + HTMLElement spanB = (HTMLElement) resultHtml.getLastElementChild(); + assertThat(spanA.getClassName_js(), is("a")); + assertThat(spanA.getInnerText(), is("T")); + assertThat(spanB.getClassName_js(), is("b")); + assertThat(spanB.getInnerText(), is("w")); + } + + @TestExtension + public static class CustomizableTLID extends TopLevelItemDescriptor { + + public String customId = "ID-not-yet-defined"; + public String customDisplayName = "DisplayName-not-yet-defined"; + public String customDescription = "Description-not-yet-defined"; + public String customIconFilePathPattern = "IconFilePathPattern-not-yet-defined"; + public String customIconClassName = "IconClassName-not-yet-defined"; + + public CustomizableTLID() { + super(FreeStyleProject.class); + } + + @Override + public String getId() { + return customId; + } + + @Override + public String getDisplayName() { + return customDisplayName; + } + + @Override + public String getDescription() { + return customDescription; + } + + @Override + public @CheckForNull String getIconFilePathPattern() { + return customIconFilePathPattern; + } + + @Override + public String getIconClassName() { + return customIconClassName; + } + + @Override + public TopLevelItem newInstance(ItemGroup parent, String name) { + throw new UnsupportedOperationException(); + } + + } + //Duplication with ViewTest.CompositeView from core unit test module - unfortunately it is inaccessible from here private static class DummyCompositeView extends View implements ViewGroup { diff --git a/test/src/test/java/jenkins/model/JenkinsSEC2047Test.java b/test/src/test/java/jenkins/model/JenkinsSEC2047Test.java deleted file mode 100644 index 3d460ddd9b..0000000000 --- a/test/src/test/java/jenkins/model/JenkinsSEC2047Test.java +++ /dev/null @@ -1,83 +0,0 @@ -package jenkins.model; - -import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; -import com.gargoylesoftware.htmlunit.html.HtmlPage; -import hudson.model.RootAction; -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.JenkinsRule.WebClient; -import org.jvnet.hudson.test.MockAuthorizationStrategy; -import org.jvnet.hudson.test.TestExtension; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.fail; - -//TODO merge back to JenkinsTest (or put it somewhere else) -public class JenkinsSEC2047Test { - - @Rule public JenkinsRule j = new JenkinsRule(); - - @Issue("SECURITY-2047") - @Test - public void testLogin123() throws Exception { - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()); - WebClient wc = j.createWebClient(); - - try { - HtmlPage login123 = wc.goTo("login123"); - fail("Page should be protected."); - } catch (FailingHttpStatusCodeException e) { - assertThat(e.getStatusCode(), is(403)); - } - } - - @Issue("SECURITY-2047") - @Test - public void testLogin123WithRead() throws Exception { - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy(). - grant(Jenkins.READ).everywhere().to("bob")); - WebClient wc = j.createWebClient(); - - wc.login("bob"); - HtmlPage login123 = wc.goTo("login123"); - assertThat(login123.getWebResponse().getStatusCode(), is(200)); - assertThat(login123.getWebResponse().getContentAsString(), containsString("This should be protected")); - } - - @Test - public void testLogin() throws Exception { - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy(). - grant(Jenkins.READ).everywhere().to("bob")); - WebClient wc = j.createWebClient(); - - HtmlPage login = wc.goTo("login"); - assertThat(login.getWebResponse().getStatusCode(), is(200)); - assertThat(login.getWebResponse().getContentAsString(), containsString("login")); - } - - @TestExtension({"testLogin123", "testLogin123WithRead"}) - public static class ProtectedRootAction implements RootAction { - @Override - public String getIconFileName() { - return "document.png"; - } - - @Override - public String getDisplayName() { - return "I am PROTECTED"; - } - - @Override - public String getUrlName() { - return "login123"; - } - } - -} diff --git a/test/src/test/java/jenkins/model/JenkinsTest.java b/test/src/test/java/jenkins/model/JenkinsTest.java index 59cbb7fa8c..164d23359f 100644 --- a/test/src/test/java/jenkins/model/JenkinsTest.java +++ b/test/src/test/java/jenkins/model/JenkinsTest.java @@ -27,6 +27,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertTrue; @@ -34,7 +35,9 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.HttpMethod; import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.TextPage; @@ -705,4 +708,63 @@ public class JenkinsTest { public void jobCreatedByInitializerIsRetained() { assertNotNull("JENKINS-47406 should exist", j.jenkins.getItem("JENKINS-47406")); } + + @Issue("SECURITY-2047") + @Test + public void testLogin123() throws Exception { + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()); + WebClient wc = j.createWebClient(); + + try { + wc.goTo("login123"); + fail("Page should be protected."); + } catch (FailingHttpStatusCodeException e) { + assertThat(e.getStatusCode(), is(403)); + } + } + + @Issue("SECURITY-2047") + @Test + public void testLogin123WithRead() throws Exception { + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy(). + grant(Jenkins.READ).everywhere().to("bob")); + WebClient wc = j.createWebClient(); + + wc.login("bob"); + HtmlPage login123 = wc.goTo("login123"); + assertThat(login123.getWebResponse().getStatusCode(), is(200)); + assertThat(login123.getWebResponse().getContentAsString(), containsString("This should be protected")); + } + + @Test + public void testLogin() throws Exception { + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy(). + grant(Jenkins.READ).everywhere().to("bob")); + WebClient wc = j.createWebClient(); + + HtmlPage login = wc.goTo("login"); + assertThat(login.getWebResponse().getStatusCode(), is(200)); + assertThat(login.getWebResponse().getContentAsString(), containsString("login")); + } + + @TestExtension({"testLogin123", "testLogin123WithRead"}) + public static class ProtectedRootAction implements RootAction { + @Override + public String getIconFileName() { + return "document.png"; + } + + @Override + public String getDisplayName() { + return "I am PROTECTED"; + } + + @Override + public String getUrlName() { + return "login123"; + } + } } diff --git a/test/src/test/java/lib/form/HeteroListSEC2035Test.java b/test/src/test/java/lib/form/HeteroListTest.java similarity index 98% rename from test/src/test/java/lib/form/HeteroListSEC2035Test.java rename to test/src/test/java/lib/form/HeteroListTest.java index c1c8abb066..0507cbba0e 100644 --- a/test/src/test/java/lib/form/HeteroListSEC2035Test.java +++ b/test/src/test/java/lib/form/HeteroListTest.java @@ -55,12 +55,11 @@ import java.util.Optional; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -//TODO rename to HeteroListTest after security release -public class HeteroListSEC2035Test { +public class HeteroListTest { @Rule public JenkinsRule j = new JenkinsRule(); diff --git a/test/src/test/resources/jenkins/model/JenkinsSEC2047Test/ProtectedRootAction/index.jelly b/test/src/test/resources/jenkins/model/JenkinsTest/ProtectedRootAction/index.jelly similarity index 100% rename from test/src/test/resources/jenkins/model/JenkinsSEC2047Test/ProtectedRootAction/index.jelly rename to test/src/test/resources/jenkins/model/JenkinsTest/ProtectedRootAction/index.jelly diff --git a/test/src/test/resources/lib/form/HeteroListSEC2035Test/RootActionImpl/index.jelly b/test/src/test/resources/lib/form/HeteroListTest/RootActionImpl/index.jelly similarity index 100% rename from test/src/test/resources/lib/form/HeteroListSEC2035Test/RootActionImpl/index.jelly rename to test/src/test/resources/lib/form/HeteroListTest/RootActionImpl/index.jelly