Merge pull request #101 from jenkinsci-cert/security-383-simpler

[SECURITY-383] Additional XStream2-specific class blacklisting
jenkinsci · Jan 23, 2017 · a814154 · a814154
1 parent 92964da
commit a814154
Show file tree

Hide file tree

Showing 3 changed files with 175 additions and 1 deletion.
diff --git a/pom.xml b/pom.xml
@@ -179,7 +179,7 @@ THE SOFTWARE.
       <dependency>
         <groupId>org.jenkins-ci.main</groupId>
         <artifactId>remoting</artifactId>
-        <version>2.53.4</version>
+        <version>2.53.5-20170111.201445-1</version>
       </dependency>
 
       <dependency>

diff --git a/test/src/test/java/hudson/util/XStream2Security383Test.java 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
@@ -0,0 +1,54 @@
+<set>
+    <hudson.util.RunList>
+        <base class="java.util.ServiceLoader">
+            <providers/>
+            <lookupIterator>
+                <configs class="java.util.Collections$3">
+                    <i class="java.util.AbstractList$Itr">
+                        <cursor>0</cursor>
+                        <lastRet>-1</lastRet>
+                        <expectedModCount>0</expectedModCount>
+                        <outer-class class="java.util.Arrays$ArrayList">
+                            <a class="string-array">
+                                <string>foo</string>
+                            </a>
+                        </outer-class>
+                    </i>
+                    <val_-c class="java.util.Arrays$ArrayList" reference="../i/outer-class"/>
+                </configs>
+                <pending class="javax.imageio.spi.FilterIterator">
+                    <iter class="java.util.AbstractList$Itr">
+                        <cursor>0</cursor>
+                        <lastRet>-1</lastRet>
+                        <expectedModCount>0</expectedModCount>
+                        <outer-class class="java.util.Arrays$ArrayList">
+                            <a>
+                                <java.lang.ProcessBuilder>
+                                    <command>
+                                        <string>touch</string>
+                                        <string>@TOKEN@</string>
+                                    </command>
+                                    <redirectErrorStream>false</redirectErrorStream>
+                                </java.lang.ProcessBuilder>
+                            </a>
+                        </outer-class>
+                    </iter>
+                    <filter class="javax.imageio.ImageIO$ContainsFilter">
+                        <method>
+                            <class>java.lang.ProcessBuilder</class>
+                            <name>start</name>
+                            <parameter-types/>
+                        </method>
+                        <name>foo</name>
+                    </filter>
+                    <next class="java.util.HashMap$Node">
+                        <hash>101575</hash>
+                        <key class="string">foo</key>
+                        <value class="string">foo</value>
+                    </next>
+                </pending>
+                <outer-class reference="../.."/>
+            </lookupIterator>
+        </base>
+    </hudson.util.RunList>
+</set>