Skip to content

Commit

Permalink
Merge pull request #336 from vithu30/master
Browse files Browse the repository at this point in the history
Fix improper restriction of XXE
  • Loading branch information
vithu30 authored Feb 17, 2020
2 parents eefc170 + a4bec1f commit ad36968
Showing 1 changed file with 32 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.xerces.util.SecurityManager;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;
Expand Down Expand Up @@ -57,6 +58,7 @@ public class GovernanceUtils {
public static final String DEFAULT_ENDPOINT_ACTIVE_DURATION = "DefaultEndpointActiveDuration";
public static final String ENABLE_LIFECYCLE_CHECKLIST_ITEMS = "enableLifecycleChecklistItems";
public static final String LIFECYCLE_CHECKLIST_ITEMS_ENABLED = "true";
private static final int ENTITY_EXPANSION_LIMIT = 0;
private static Log log = LogFactory.getLog(GovernanceUtils.class);


Expand Down Expand Up @@ -93,7 +95,7 @@ public static GovernanceConfiguration getGovernanceConfiguration() throws Govern
private static void initGovernanceConfiguration(InputStream in, GovernanceConfiguration govConfig)
throws GovernanceConfigurationException {
try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
DocumentBuilderFactory factory = getSecuredDocumentBuilder();
DocumentBuilder builder = factory.newDocumentBuilder();
Document document = builder.parse(in);
readChildElements(document.getDocumentElement(), govConfig);
Expand Down Expand Up @@ -220,5 +222,34 @@ public static String getCarbonConfigDirPath() {
}
return carbonConfigDir;
}

/**
* Returns a secured DocumentBuilderFactory instance
*
* @return DocumentBuilderFactory
*/
public static DocumentBuilderFactory getSecuredDocumentBuilder() {

org.apache.xerces.impl.Constants Constants = null;
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setNamespaceAware(true);
dbf.setXIncludeAware(false);
dbf.setExpandEntityReferences(false);
try {
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);
} catch (ParserConfigurationException e) {
log.error(
"Failed to load XML Processor Feature " + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE + " or " +
Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE + " or " + Constants.LOAD_EXTERNAL_DTD_FEATURE);
}

SecurityManager securityManager = new SecurityManager();
securityManager.setEntityExpansionLimit(ENTITY_EXPANSION_LIMIT);
dbf.setAttribute(Constants.XERCES_PROPERTY_PREFIX + Constants.SECURITY_MANAGER_PROPERTY, securityManager);

return dbf;
}
}

0 comments on commit ad36968

Please # to comment.