CVE-2016-0792

@@ -47,6 +47,7 @@
 import hudson.PluginManager;
 import hudson.PluginWrapper;
 import hudson.diagnosis.OldDataMonitor;
+import hudson.remoting.ClassFilter;
 import hudson.util.xstream.ImmutableSetConverter;
 import hudson.util.xstream.ImmutableSortedSetConverter;
 import jenkins.model.Jenkins;
@@ -159,6 +160,8 @@ private void init() {
         // but before reflection-based one kicks in.
         registerConverter(new AssociatedConverterImpl(this), -10);
 
+        registerConverter(new BlacklistedTypesConverter(), PRIORITY_VERY_HIGH); // SECURITY-247 defense
+
         registerConverter(new DynamicProxyConverter(getMapper()) { // SECURITY-105 defense
             @Override public boolean canConvert(Class type) {
                 return /* this precedes NullConverter */ type != null && super.canConvert(type);
@@ -434,4 +437,30 @@ class PluginClassOwnership implements ClassOwnership {
 
     }
 
+    private static class BlacklistedTypesConverter implements Converter {
+        @Override
+        public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingContext context) {
+            throw new UnsupportedOperationException("Refusing to marshal for security reasons");
+        }
+
+        @Override
+        public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) {
+            throw new ConversionException("Refusing to unmarshal for security reasons");
+        }
+
+        @Override
+        public boolean canConvert(Class type) {
+            if (type == null) {
+                return false;
+            }
+            try {
+                ClassFilter.DEFAULT.check(type);
+                ClassFilter.DEFAULT.check(type.getName());
+            } catch (SecurityException se) {
+                // claim we can convert all the scary stuff so we can throw exceptions when attempting to do so
+                return true;
+            }
+            return false;
+        }
+    }
 }

@@ -0,0 +1,120 @@
+package hudson.util;
+
+import hudson.model.Items;
+import org.apache.commons.io.*;
+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 XStream2Security247Test {
+
+    @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-247")
+    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(), "security247");
+
+            String exploitXml = org.apache.commons.io.IOUtils.toString(
+                    XStream2Security247Test.class.getResourceAsStream(
+                            "/hudson/util/XStream2Security247Test/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-247")
+    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(), "security247");
+
+            String exploitXml = org.apache.commons.io.IOUtils.toString(
+                    XStream2Security247Test.class.getResourceAsStream(
+                            "/hudson/util/XStream2Security247Test/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();
+        }
+    }
+}

@@ -0,0 +1,27 @@
+<map>
+    <entry>
+        <groovy.util.Expando>
+            <expandoProperties>
+                <entry>
+                    <string>hashCode</string>
+                    <org.codehaus.groovy.runtime.MethodClosure>
+                        <delegate class="groovy.util.Expando" reference="../../../.."/>
+                        <owner class="java.lang.ProcessBuilder">
+                            <command>
+                                <string>touch</string>
+                                <string>@TOKEN@</string>
+                            </command>
+                            <redirectErrorStream>false</redirectErrorStream>
+                        </owner>
+                        <resolveStrategy>0</resolveStrategy>
+                        <directive>0</directive>
+                        <parameterTypes/>
+                        <maximumNumberOfParameters>0</maximumNumberOfParameters>
+                        <method>start</method>
+                    </org.codehaus.groovy.runtime.MethodClosure>
+                </entry>
+            </expandoProperties>
+        </groovy.util.Expando>
+        <int>1</int>
+    </entry>
+</map>