From a814154695e23dc37542af7d40cacc129cf70722 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 12 Jan 2017 16:36:43 -0500 Subject: [PATCH] Merge pull request #101 from jenkinsci-cert/security-383-simpler [SECURITY-383] Additional XStream2-specific class blacklisting --- pom.xml | 2 +- .../hudson/util/XStream2Security383Test.java | 120 ++++++++++++++++++ .../util/XStream2Security383Test/config.xml | 54 ++++++++ 3 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 test/src/test/java/hudson/util/XStream2Security383Test.java create mode 100644 test/src/test/resources/hudson/util/XStream2Security383Test/config.xml diff --git a/pom.xml b/pom.xml index f57a3b6345f0..f01684c3f140 100644 --- a/pom.xml +++ b/pom.xml @@ -179,7 +179,7 @@ THE SOFTWARE. org.jenkins-ci.main remoting - 2.53.4 + 2.53.5-20170111.201445-1 diff --git a/test/src/test/java/hudson/util/XStream2Security383Test.java b/test/src/test/java/hudson/util/XStream2Security383Test.java new file mode 100644 index 000000000000..1e3897a5f220 --- /dev/null +++ b/test/src/test/java/hudson/util/XStream2Security383Test.java @@ -0,0 +1,120 @@ +package hudson.util; + +import hudson.model.Items; +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.IOUtils; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import javax.servlet.ServletInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; + +import static org.junit.Assert.assertFalse; +import static org.mockito.Mockito.when; + +public class XStream2Security383Test { + + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Rule + public TemporaryFolder f = new TemporaryFolder(); + + @Mock + private StaplerRequest req; + + @Mock + private StaplerResponse rsp; + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + } + + @Test + @Issue("SECURITY-383") + public void testXmlLoad() throws Exception { + File exploitFile = f.newFile(); + try { + // be extra sure there's no file already + if (exploitFile.exists() && !exploitFile.delete()) { + throw new IllegalStateException("file exists and cannot be deleted"); + } + File tempJobDir = new File(j.jenkins.getRootDir(), "security383"); + + String exploitXml = IOUtils.toString( + XStream2Security383Test.class.getResourceAsStream( + "/hudson/util/XStream2Security383Test/config.xml"), "UTF-8"); + + exploitXml = exploitXml.replace("@TOKEN@", exploitFile.getAbsolutePath()); + + FileUtils.write(new File(tempJobDir, "config.xml"), exploitXml); + + try { + Items.load(j.jenkins, tempJobDir); + } catch (Exception e) { + // ignore + } + assertFalse("no file should be created here", exploitFile.exists()); + } finally { + exploitFile.delete(); + } + } + + @Test + @Issue("SECURITY-383") + public void testPostJobXml() throws Exception { + File exploitFile = f.newFile(); + try { + // be extra sure there's no file already + if (exploitFile.exists() && !exploitFile.delete()) { + throw new IllegalStateException("file exists and cannot be deleted"); + } + File tempJobDir = new File(j.jenkins.getRootDir(), "security383"); + + String exploitXml = IOUtils.toString( + XStream2Security383Test.class.getResourceAsStream( + "/hudson/util/XStream2Security383Test/config.xml"), "UTF-8"); + + exploitXml = exploitXml.replace("@TOKEN@", exploitFile.getAbsolutePath()); + + when(req.getMethod()).thenReturn("POST"); + when(req.getInputStream()).thenReturn(new Stream(IOUtils.toInputStream(exploitXml))); + when(req.getContentType()).thenReturn("application/xml"); + when(req.getParameter("name")).thenReturn("foo"); + + try { + j.jenkins.doCreateItem(req, rsp); + } catch (Exception e) { + // don't care + } + + assertFalse("no file should be created here", exploitFile.exists()); + } finally { + exploitFile.delete(); + } + } + + private static class Stream extends ServletInputStream { + private final InputStream inner; + + public Stream(final InputStream inner) { + this.inner = inner; + } + + @Override + public int read() throws IOException { + return inner.read(); + } + } +} diff --git a/test/src/test/resources/hudson/util/XStream2Security383Test/config.xml b/test/src/test/resources/hudson/util/XStream2Security383Test/config.xml new file mode 100644 index 000000000000..d7edab376442 --- /dev/null +++ b/test/src/test/resources/hudson/util/XStream2Security383Test/config.xml @@ -0,0 +1,54 @@ + + + + + + + + 0 + -1 + 0 + + + foo + + + + + + + + 0 + -1 + 0 + + + + + touch + @TOKEN@ + + false + + + + + + + java.lang.ProcessBuilder + start + + + foo + + + 101575 + foo + foo + + + + + + + \ No newline at end of file