From e295711343cc155cb64ea0ae29ce9d69201469b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?=
 <juansotuyo@gmail.com>
Date: Sun, 3 Sep 2017 21:12:15 -0300
Subject: [PATCH 1/2] Harden XML parsing

 - Fixes #532
---
 .../net/sourceforge/pmd/RuleSetFactory.java   | 42 +++++++++++++++++--
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java
index 86040feab7..1eb4bda27d 100644
--- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java
+++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java
@@ -331,7 +331,7 @@ public class RuleSetFactory {
                 throw new IllegalArgumentException(
                         "Cannot parse a RuleSet from a non-external reference: <" + ruleSetReferenceId + ">.");
             }
-            DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
+            DocumentBuilder builder = createDocumentBuilder();
             InputSource inputSource;
             if (compatibilityFilter != null) {
                 inputSource = new InputSource(compatibilityFilter.filterRuleSetFile(inputStream));
@@ -392,6 +392,42 @@ public class RuleSetFactory {
         }
     }
 
+    private DocumentBuilder createDocumentBuilder() throws ParserConfigurationException {
+        final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+        
+        try {
+            /*
+             * parser hardening
+             * https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Prevention_Cheat_Sheet#JAXP_DocumentBuilderFactory.2C_SAXParserFactory_and_DOM4J
+             */
+            // This is the PRIMARY defense. If DTDs (doctypes) are disallowed, almost all XML entity attacks are prevented
+            // Xerces 2 only - http://xerces.apache.org/xerces2-j/features.html#disallow-doctype-decl
+            dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+            
+            // If you can't completely disable DTDs, then at least do the following:
+            // Xerces 1 - http://xerces.apache.org/xerces-j/features.html#external-general-entities
+            // Xerces 2 - http://xerces.apache.org/xerces2-j/features.html#external-general-entities
+            // JDK7+ - http://xml.org/sax/features/external-general-entities    
+            dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            
+            // Xerces 1 - http://xerces.apache.org/xerces-j/features.html#external-parameter-entities
+            // Xerces 2 - http://xerces.apache.org/xerces2-j/features.html#external-parameter-entities
+            // JDK7+ - http://xml.org/sax/features/external-parameter-entities    
+            dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            
+            // Disable external DTDs as well
+            dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            
+            // and these as well, per Timothy Morgan's 2014 paper: "XML Schema, DTD, and Entity Attacks"
+            dbf.setXIncludeAware(false);
+            dbf.setExpandEntityReferences(false);
+        } catch (final ParserConfigurationException e) {
+            // an unsupported feature... too bad, but won't fail execution due to this
+        }
+        
+        return dbf.newDocumentBuilder();
+    }
+
     private static RuleSet classNotFoundProblem(Exception ex) {
         ex.printStackTrace();
         throw new RuntimeException("Couldn't find the class " + ex.getMessage());
@@ -736,7 +772,7 @@ public class RuleSetFactory {
     private boolean containsRule(RuleSetReferenceId ruleSetReferenceId, String ruleName) {
         boolean found = false;
         try (InputStream ruleSet = ruleSetReferenceId.getInputStream(classLoader)) {
-            DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
+            DocumentBuilder builder = createDocumentBuilder();
             Document document = builder.parse(ruleSet);
             Element ruleSetElement = document.getDocumentElement();
 
@@ -859,7 +895,7 @@ public class RuleSetFactory {
         }
 
         // casting is not pretty but prevents the interface from having this method
-        PropertyDescriptor<?> desc = ((AbstractPropertyDescriptorFactory) pdFactory).createExternalWith(values);
+        PropertyDescriptor<?> desc = ((AbstractPropertyDescriptorFactory<?>) pdFactory).createExternalWith(values);
 
         rule.definePropertyDescriptor(desc);
         setValue(rule, desc, strValue);

From 3023a8274d1e752d0e17a9a4f13bec94a9cb7ac3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?=
 <juansotuyo@gmail.com>
Date: Sun, 3 Sep 2017 21:23:09 -0300
Subject: [PATCH 2/2] Update changelog, refs #592

---
 docs/pages/release_notes.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md
index 3c41b2e60c..3db37b50f1 100644
--- a/docs/pages/release_notes.md
+++ b/docs/pages/release_notes.md
@@ -139,6 +139,8 @@ All existing rules have been updated to reflect these changes. If you have custo
 
 ### Fixed Issues
 
+*   all
+    *  [#532](https://github.com/pmd/pmd/issues/532): \[core] security concerns on URL-based rulesets
 *   apex
     *   [#488](https://github.com/pmd/pmd/pull/488): \[apex] Use Apex lexer for CPD
     *   [#489](https://github.com/pmd/pmd/pull/489): \[apex] Update Apex compiler