[SECURITY-167] attempt to set JAXP properties.

Atempt to set SAX specific properties to defend against XXE attacks.

core/src/main/java/jenkins/util/xml/XMLUtils.java

@@ -41,7 +41,18 @@ public final class XMLUtils {
             stFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
 
             XMLReader xmlReader = XMLReaderFactory.createXMLReader();
+            try {
+                xmlReader.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            }
+            catch (SAXException ignored) { /* ignored */ }
+            try {
+                xmlReader.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            }
+            catch (SAXException ignored) { /* ignored */ }
             // defend against XXE
+            // the above features should strip out entities - however the feature may not be supported depending
+            // on the xml implementation used and this is out of our control.
+            // So add a fallback plan if all else fails.
             xmlReader.setEntityResolver(RestrictiveEntityResolver.INSTANCE);
             SAXSource saxSource = new SAXSource(xmlReader, src);
             _transform(saxSource, out);

core/src/test/java/jenkins/xml/XMLUtilsTest.java

@@ -30,6 +30,7 @@ import org.apache.commons.io.output.NullOutputStream;
 import org.junit.Test;
 
 import java.io.StringReader;
+import java.io.StringWriter;
 import javax.xml.transform.TransformerException;
 import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
@@ -61,9 +62,11 @@ public class XMLUtilsTest {
                 "</project>";
 
 
+        StringWriter stringWriter = new StringWriter();
         try {
-            XMLUtils.safeTransform(new StreamSource(new StringReader(xml)), new StreamResult(new NullOutputStream()));
-            fail("Exception should have been thrown");
+            XMLUtils.safeTransform(new StreamSource(new StringReader(xml)), new StreamResult(stringWriter));
+            // if no exception then JAXP is swallowing these - so there should be no entity in the description.
+            assertThat(stringWriter.toString(), containsString("<description/>"));
         } catch (TransformerException ex) {
             assertThat(ex.getMessage(), containsString("Refusing to resolve entity"));
         }
@@ -88,7 +91,11 @@ public class XMLUtilsTest {
                 "  <buildWrappers/>\n" +
                 "</project>";
 
-        XMLUtils.safeTransform(new StreamSource(new StringReader(xml)), new StreamResult(new NullOutputStream()));
+        StringWriter stringWriter = new StringWriter();
+
+        XMLUtils.safeTransform(new StreamSource(new StringReader(xml)), new StreamResult(stringWriter));
+        // make sure that normal entities are retained.
+        assertThat(stringWriter.toString(), containsString("<description>&amp;</description>"));
     }
 
 }