common.core: improve logging of documentBuilder hardenning
If one feature fails to set, try the next ones anyway.
[Security] Improve XML documentBuilder hardenning
Change-Id: Iec7c14c1824a8ce666e93c8b15a73d585f52d718
Signed-off-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Reviewed-on: https://git.eclipse.org/r/c/tracecompass/org.eclipse.tracecompass/+/182950
Tested-by: Trace Compass Bot <tracecompass-bot@eclipse.org>
Tested-by: Patrick Tasse <patrick.tasse@gmail.com>
Reviewed-by: Patrick Tasse <patrick.tasse@gmail.com>
diff --git a/common/org.eclipse.tracecompass.common.core/src/org/eclipse/tracecompass/common/core/xml/XmlUtils.java b/common/org.eclipse.tracecompass.common.core/src/org/eclipse/tracecompass/common/core/xml/XmlUtils.java
index ee95819..7c3ed8a 100644
--- a/common/org.eclipse.tracecompass.common.core/src/org/eclipse/tracecompass/common/core/xml/XmlUtils.java
+++ b/common/org.eclipse.tracecompass.common.core/src/org/eclipse/tracecompass/common/core/xml/XmlUtils.java
@@ -13,6 +13,8 @@
import java.io.IOException;
import java.io.InputStream;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilderFactory;
@@ -30,7 +32,8 @@
import javax.xml.validation.Validator;
import org.eclipse.jdt.annotation.Nullable;
-import org.eclipse.tracecompass.internal.common.core.Activator;
+import org.eclipse.tracecompass.common.core.log.TraceCompassLog;
+import org.eclipse.tracecompass.common.core.log.TraceCompassLogUtils;
import org.xml.sax.SAXException;
import org.xml.sax.SAXNotRecognizedException;
import org.xml.sax.SAXNotSupportedException;
@@ -44,6 +47,8 @@
*/
public final class XmlUtils {
+ private static final Logger LOGGER = TraceCompassLog.getLogger(XmlUtils.class);
+
private static final String EMPTY = ""; //$NON-NLS-1$
private static final String DISALLOW_DOCTYPE_DECL = "http://apache.org/xml/features/disallow-doctype-decl"; //$NON-NLS-1$
private static final String NONVALIDATING_LOAD_EXTERNAL_DTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd"; //$NON-NLS-1$
@@ -93,7 +98,12 @@
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
try {
// This one is from Sonar (squid:S2755)
- dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+ feature = XMLConstants.FEATURE_SECURE_PROCESSING;
+ dbf.setFeature(feature, true);
+ } catch (ParserConfigurationException e) {
+ logException(feature, e);
+ }
+ try {
// This is the PRIMARY defense. If DTDs (doctypes) are disallowed,
// almost all
// XML entity attacks are prevented
@@ -102,6 +112,10 @@
feature = DISALLOW_DOCTYPE_DECL;
dbf.setFeature(feature, true);
+ } catch (ParserConfigurationException e) {
+ logException(feature, e);
+ }
+ try {
// If you can't completely disable DTDs, then at least do the
// following:
// Xerces 1 -
@@ -111,7 +125,10 @@
// JDK7+ - http://xml.org/sax/features/external-general-entities
feature = ACCESS_EXTERNAL_GENERAL_ENTITIES;
dbf.setFeature(feature, false);
-
+ } catch (ParserConfigurationException e) {
+ logException(feature, e);
+ }
+ try {
// Xerces 1 -
// http://xerces.apache.org/xerces-j/features.html#external-parameter-entities
// Xerces 2 -
@@ -119,31 +136,28 @@
// JDK7+ - http://xml.org/sax/features/external-parameter-entities
feature = USE_EXTERNAL_PARAMETER_ENTITIES;
dbf.setFeature(feature, false);
-
+ } catch (ParserConfigurationException e) {
+ logException(feature, e);
+ }
+ try {
// Disable external DTDs as well
feature = NONVALIDATING_LOAD_EXTERNAL_DTD;
dbf.setFeature(feature, false);
-
- // and these as well, per Timothy Morgan's 2014 paper: "XML Schema,
- // DTD, and Entity Attacks"
- dbf.setXIncludeAware(false);
- dbf.setExpandEntityReferences(false);
-
- // And, per Timothy Morgan:
- // "If for some reason support for inline DOCTYPEs are a requirement, then ensure
- // the entity settings are disabled (as shown above) and beware that SSRF attacks
- // (http://cwe.mitre.org/data/definitions/918.html)
- // and denial of service attacks (such as billion laughs or decompression bombs via
- // "jar:") are a risk."
-
} catch (ParserConfigurationException e) {
- // This should catch a failed setFeature feature
- Activator.instance().logInfo("ParserConfigurationException was thrown. The feature '" + feature //$NON-NLS-1$
- + "' is probably not supported by your XML processor.", e); //$NON-NLS-1$
- }
+ logException(feature, e);
+ } // and these as well, per Timothy Morgan's 2014 paper: "XML Schema,
+ // DTD, and Entity Attacks"
+ dbf.setXIncludeAware(false);
+ dbf.setExpandEntityReferences(false);
+
return dbf;
}
+ private static void logException(String feature, ParserConfigurationException e) {
+ // This should catch a failed setFeature feature
+ TraceCompassLogUtils.traceInstant(LOGGER, Level.WARNING, "ParserConfigurationException" , "feature", feature, "stack", e.getStackTrace()); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+ }
+
/**
* Create a new safe {@link XMLStreamReader} from an {@link InputStream}
*
