Merge pull request #12526 from RusJaI/master

Code level fixes for security enhancements
wso2 · Sep 7, 2024 · 41bf4fe · 41bf4fe
2 parents 56d2b9a + 9c8490d
commit 41bf4fe
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 6 deletions.
diff --git a/...so2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/OperationPolicy.java b/...so2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/OperationPolicy.java
@@ -95,12 +95,13 @@ public boolean equals(Object o) {
 
         if (this == o)
             return true;
-        if (o == null || getClass() != o.getClass())
-            return false;
-        OperationPolicy policyObj = (OperationPolicy) o;
-        return Objects.equals(policyName, policyObj.policyName) && Objects.equals(policyVersion,
-                policyObj.policyVersion) && Objects.equals(direction, policyObj.direction) && Objects.equals(
-                parameters, policyObj.parameters) && Objects.equals(policyId, policyObj.policyId);
+        if (o instanceof OperationPolicy) {
+            OperationPolicy policyObj = (OperationPolicy) o;
+            return Objects.equals(policyName, policyObj.policyName) && Objects.equals(policyVersion,
+                    policyObj.policyVersion) && Objects.equals(direction, policyObj.direction) && Objects.equals(
+                    parameters, policyObj.parameters) && Objects.equals(policyId, policyObj.policyId);
+        }
+        return false;
     }
 
     @Override

diff --git a/...t/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java b/...t/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java
@@ -27,6 +27,7 @@
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.xerces.impl.Constants;
 import org.everit.json.schema.Schema;
 import org.everit.json.schema.ValidationException;
 import org.json.JSONException;
@@ -111,9 +112,11 @@
 import java.util.UUID;
 import java.util.stream.Collectors;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.OutputKeys;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerException;
 import javax.xml.transform.TransformerFactory;
@@ -1066,6 +1069,22 @@ public String extractBotDetectionDataContent(String messageBody) {
         try {
             //Parse the message body and extract the content in XML form
             DocumentBuilderFactory factory = APIUtil.getSecuredDocumentBuilder();
+
+            factory.setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.DISALLOW_DOCTYPE_DECL_FEATURE,
+                    true);
+
+            // Enable secure processing
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+
+            // Enable namespace awareness
+            factory.setNamespaceAware(true);
+
+            // Disable external entities to prevent XXE attacks
+            factory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE,
+                    false);
+            factory.setFeature(Constants.SAX_FEATURE_PREFIX +
+                    Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE, false);
+
             DocumentBuilder builder = factory.newDocumentBuilder();
             Document document = builder.parse(new InputSource(new StringReader(messageBody)));
             Node bodyContentNode = document.getFirstChild().getFirstChild();
@@ -1074,6 +1093,7 @@ public String extractBotDetectionDataContent(String messageBody) {
             if (bodyContentNode != null) {
                 StringWriter writer = new StringWriter();
                 Transformer transformer = TransformerFactory.newInstance().newTransformer();
+                transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes");
                 transformer.transform(new DOMSource(bodyContentNode), new StreamResult(writer));
                 String output = writer.toString();
                 content = output.substring(output.indexOf("?>") + 2); //remove <?xml version="1.0" encoding="UTF-8"?>

diff --git a/...o2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIMWSDLReader.java b/...o2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIMWSDLReader.java
@@ -310,6 +310,21 @@ public Document getSecuredParsedDocument(String file) throws APIManagementExcept
         try {
             inputStream = new FileInputStream(new File(file));
             DocumentBuilderFactory factory = getSecuredDocumentBuilder();
+            // Enable secure processing
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+
+            // Enable namespace awareness
+            factory.setNamespaceAware(true);
+
+            factory.setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.DISALLOW_DOCTYPE_DECL_FEATURE,
+                    true);
+
+            // Disable external entities to prevent XXE attacks
+            factory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE,
+                    false);
+            factory.setFeature(Constants.SAX_FEATURE_PREFIX +
+                    Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE, false);
+
             DocumentBuilder builder = factory.newDocumentBuilder();
             return builder.parse(inputStream);
         } catch (ParserConfigurationException | IOException | SAXException e) {

diff --git a/.../org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java b/.../org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java
@@ -288,6 +288,7 @@
 import javax.cache.Caching;
 import java.security.cert.X509Certificate;
 import javax.validation.constraints.NotNull;
+import javax.xml.XMLConstants;
 import javax.xml.namespace.QName;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -6843,6 +6844,9 @@ public static DocumentBuilderFactory getSecuredDocumentBuilder() {
         dbf.setXIncludeAware(false);
         dbf.setExpandEntityReferences(false);
         try {
+            // Enable secure processing
+            dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            dbf.setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.DISALLOW_DOCTYPE_DECL_FEATURE, true);
             dbf.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE, false);
             dbf.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE, false);
             dbf.setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.LOAD_EXTERNAL_DTD_FEATURE, false);

diff --git a/...rbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/CertificateMgtUtils.java b/...rbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/CertificateMgtUtils.java
@@ -27,6 +27,7 @@
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.xerces.impl.Constants;
 import org.jetbrains.annotations.NotNull;
 import org.w3c.dom.Document;
 import org.wso2.carbon.apimgt.api.dto.CertificateInformationDTO;
@@ -73,6 +74,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
+import javax.xml.XMLConstants;
 import javax.xml.namespace.QName;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -853,6 +855,14 @@ private static void updateSenderProfileTrustStoreLocation(String jksLocation)
                 try {
                     String xml = customSSLProfilesOmElement.toString();
                     DocumentBuilderFactory factory = APIUtil.getSecuredDocumentBuilder();
+                    factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+                    factory.setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.DISALLOW_DOCTYPE_DECL_FEATURE,
+                            true);
+                    factory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE,
+                            false);
+                    factory.setFeature(Constants.SAX_FEATURE_PREFIX +
+                            Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE, false);
+                    factory.setNamespaceAware(true);
                     DocumentBuilder builder = factory.newDocumentBuilder();
                     Document doc = builder.parse(new InputSource(new StringReader(xml)));
                     DOMSource source = new DOMSource(doc);

diff --git a/...bon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/wsdl/AbstractWSDLProcessor.java b/...bon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/wsdl/AbstractWSDLProcessor.java
@@ -124,6 +124,7 @@ private DocumentBuilderFactory getSecuredDocumentBuilder() {
         dbf.setXIncludeAware(false);
         dbf.setExpandEntityReferences(false);
         try {
+            dbf.setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.DISALLOW_DOCTYPE_DECL_FEATURE, true);
             dbf.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE, false);
             dbf.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE, false);
             dbf.setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.LOAD_EXTERNAL_DTD_FEATURE, false);

diff --git a/....carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/wsdl/SequenceGenerator.java b/....carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/wsdl/SequenceGenerator.java
@@ -39,6 +39,7 @@
 import io.swagger.util.Yaml;
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.xerces.impl.Constants;
 import org.json.JSONException;
 import org.json.simple.parser.JSONParser;
 import org.json.simple.parser.ParseException;
@@ -371,6 +372,10 @@ private static Map<String, String> createPayloadFacXMLForOperation(Map<String, S
         try {
             transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
             Transformer transformer = transformerFactory.newTransformer();
+            docFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE,
+                    false);
+            docFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE,
+                    false);
             docBuilder = docFactory.newDocumentBuilder();
             Document doc = docBuilder.newDocument();
             Element rootElement = null;
@@ -581,6 +586,10 @@ private static String createParameterElements(String jsonPathElement, String typ
             transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
             Transformer transformer = transformerFactory.newTransformer();
             docBuilder = docFactory.newDocumentBuilder();
+            docFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE,
+                    false);
+            docFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE,
+                    false);
             Document doc = docBuilder.newDocument();
             Element argElement = doc.createElement(SOAPToRESTConstants.SequenceGen.ARG_ELEMENT);
             Element propertyElement = doc.createElement(SOAPToRESTConstants.SequenceGen.PROPERTY_ELEMENT);