[FIX SECURITY-247] Prevent loading of MethodClosure from XML

core/src/main/java/hudson/util/XStream2.java

@@ -159,6 +159,8 @@ public class XStream2 extends XStream {
         // 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 +436,20 @@ public class XStream2 extends XStream {
 
     }
 
+    private static class BlacklistedTypesConverter implements Converter {
+        @Override
+        public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingContext context) {
+            throw new UnsupportedOperationException("Cannot marshal MethodClosure");
+        }
+
+        @Override
+        public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) {
+            throw new ConversionException("Cannot load MethodClosure for security reasons");
+        }
+
+        @Override
+        public boolean canConvert(Class type) {
+            return type != null && "org.codehaus.groovy.runtime.MethodClosure".equals(type.getName());
+        }
+    }
 }

test/src/test/java/hudson/util/XStream2Security247Test.java

+package hudson.util;
+
+import hudson.Functions;
+import hudson.model.Items;
+import org.apache.commons.io.FileUtils;
+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 static org.junit.Assert.assertFalse;
+
+public class XStream2Security247Test {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Test
+    @Issue("SECURITY-247")
+    public void dontUnmarshalMethodClosure() throws Exception {
+        if (Functions.isWindows())  return;
+        File exploitFile = new File("/tmp/jenkins-security247test");
+        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");
+            FileUtils.copyInputStreamToFile(XStream2Security247Test.class.getResourceAsStream("/hudson/util/XStream2Security247Test/config.xml"),
+                    new File(tempJobDir, "config.xml"));
+            try {
+                Items.load(j.jenkins, tempJobDir);
+            } catch (Exception e) {
+                // ignore
+            }
+            assertFalse("no file should be created here", exploitFile.exists());
+        } finally {
+            exploitFile.delete();
+        }
+    }
+}

test/src/test/resources/hudson/util/XStream2Security247Test/config.xml

+<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>/tmp/jenkins-security247test</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>