[PPP-3506] - XXE Security Vulnerability in xml parsers

pentaho · Oct 20, 2016 · 2ac467c · 2ac467c
1 parent c6bf5f5
commit 2ac467c
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 8 deletions.
diff --git a/core/src/org/pentaho/di/core/xml/XMLParserFactoryProducer.java b/core/src/org/pentaho/di/core/xml/XMLParserFactoryProducer.java
@@ -22,6 +22,11 @@
 
 package org.pentaho.di.core.xml;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.dom4j.io.SAXReader;
+import org.xml.sax.EntityResolver;
+import org.xml.sax.SAXException;
 import org.xml.sax.SAXNotRecognizedException;
 import org.xml.sax.SAXNotSupportedException;
 
@@ -32,6 +37,7 @@
 
 public class XMLParserFactoryProducer {
 
+  private static final Log logger = LogFactory.getLog( XMLParserFactoryProducer.class );
   /**
    * Creates an instance of {@link DocumentBuilderFactory} class with enabled {@link XMLConstants#FEATURE_SECURE_PROCESSING} property.
    * Enabling this feature prevents from some XXE attacks (e.g. XML bomb)
@@ -43,6 +49,7 @@ public class XMLParserFactoryProducer {
   public static DocumentBuilderFactory createSecureDocBuilderFactory() throws ParserConfigurationException {
     DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
     docBuilderFactory.setFeature( XMLConstants.FEATURE_SECURE_PROCESSING, true );
+    docBuilderFactory.setFeature( "http://apache.org/xml/features/disallow-doctype-decl", true );
 
     return docBuilderFactory;
   }
@@ -65,7 +72,28 @@ public static SAXParserFactory createSecureSAXParserFactory()
     throws SAXNotSupportedException, SAXNotRecognizedException, ParserConfigurationException {
     SAXParserFactory factory = SAXParserFactory.newInstance();
     factory.setFeature( XMLConstants.FEATURE_SECURE_PROCESSING, true );
+    factory.setFeature( "http://xml.org/sax/features/external-general-entities", false );
+    factory.setFeature( "http://xml.org/sax/features/external-parameter-entities", false );
+    factory.setFeature( "http://apache.org/xml/features/nonvalidating/load-external-dtd", false );
 
     return factory;
   }
+
+  public static SAXReader getSAXReader( final EntityResolver resolver ) {
+    SAXReader reader = new SAXReader();
+    if ( resolver != null ) {
+      reader.setEntityResolver( resolver );
+    }
+    try {
+      reader.setFeature( XMLConstants.FEATURE_SECURE_PROCESSING, true );
+      reader.setFeature( "http://xml.org/sax/features/external-general-entities", false );
+      reader.setFeature( "http://xml.org/sax/features/external-parameter-entities", false );
+      reader.setFeature( "http://apache.org/xml/features/nonvalidating/load-external-dtd", false );
+    } catch ( SAXException e ) {
+      logger.error( "Some parser properties are not supported." );
+    }
+    reader.setIncludeExternalDTDDeclarations( false );
+    reader.setIncludeInternalDTDDeclarations( false );
+    return reader;
+  }
 }
diff --git a/plugins/kettle-xml-plugin/src/org/pentaho/di/trans/steps/getxmldata/GetXMLData.java b/plugins/kettle-xml-plugin/src/org/pentaho/di/trans/steps/getxmldata/GetXMLData.java
@@ -54,6 +54,7 @@
 import org.pentaho.di.core.row.ValueMetaInterface;
 import org.pentaho.di.core.row.value.ValueMetaFactory;
 import org.pentaho.di.core.vfs.KettleVFS;
+import org.pentaho.di.core.xml.XMLParserFactoryProducer;
 import org.pentaho.di.i18n.BaseMessages;
 import org.pentaho.di.trans.Trans;
 import org.pentaho.di.trans.TransMeta;
@@ -87,9 +88,8 @@ protected boolean setDocument( String StringXML, FileObject file, boolean IsInXM
     this.prevRow = buildEmptyRow(); // pre-allocate previous row
 
     try {
-      SAXReader reader = new SAXReader();
+      SAXReader reader = XMLParserFactoryProducer.getSAXReader( null );
       data.stopPruning = false;
-
       // Validate XML against specified schema?
       if ( meta.isValidating() ) {
         reader.setValidation( true );

diff --git a/...ml-plugin/src/org/pentaho/di/ui/trans/steps/getxmldata/LoopNodesImportProgressDialog.java b/...ml-plugin/src/org/pentaho/di/ui/trans/steps/getxmldata/LoopNodesImportProgressDialog.java
@@ -39,6 +39,7 @@
 import org.eclipse.swt.widgets.Shell;
 import org.pentaho.di.core.util.Utils;
 import org.pentaho.di.core.vfs.KettleVFS;
+import org.pentaho.di.core.xml.XMLParserFactoryProducer;
 import org.pentaho.di.i18n.BaseMessages;
 import org.pentaho.di.trans.steps.getxmldata.GetXMLDataMeta;
 import org.pentaho.di.trans.steps.getxmldata.IgnoreDTDEntityResolver;
@@ -134,7 +135,7 @@ private String[] doScan( IProgressMonitor monitor ) throws Exception {
     monitor.beginTask( BaseMessages.getString( PKG, "GetXMLDateLoopNodesImportProgressDialog.Task.ScanningFile",
         filename ), 1 );
 
-    SAXReader reader = new SAXReader();
+    SAXReader reader = XMLParserFactoryProducer.getSAXReader( null );
     monitor.worked( 1 );
     if ( monitor.isCanceled() ) {
       return null;

diff --git a/...ugin/src/org/pentaho/di/ui/trans/steps/getxmldata/XMLInputFieldsImportProgressDialog.java b/...ugin/src/org/pentaho/di/ui/trans/steps/getxmldata/XMLInputFieldsImportProgressDialog.java
@@ -44,6 +44,7 @@
 import org.pentaho.di.core.util.Utils;
 import org.pentaho.di.core.RowMetaAndData;
 import org.pentaho.di.core.vfs.KettleVFS;
+import org.pentaho.di.core.xml.XMLParserFactoryProducer;
 import org.pentaho.di.i18n.BaseMessages;
 import org.pentaho.di.trans.steps.getxmldata.GetXMLDataField;
 import org.pentaho.di.trans.steps.getxmldata.GetXMLDataMeta;
@@ -154,7 +155,7 @@ private RowMetaAndData[] doScan( IProgressMonitor monitor ) throws Exception {
     monitor.beginTask( BaseMessages.getString( PKG, "GetXMLDateLoopNodesImportProgressDialog.Task.ScanningFile",
         filename ), 1 );
 
-    SAXReader reader = new SAXReader();
+    SAXReader reader = XMLParserFactoryProducer.getSAXReader( null );
     monitor.worked( 1 );
     if ( monitor.isCanceled() ) {
       return null;

diff --git a/...xml-input-stream/src/org/pentaho/di/trans/steps/xmlinputsax/XMLInputSaxDataRetriever.java b/...xml-input-stream/src/org/pentaho/di/trans/steps/xmlinputsax/XMLInputSaxDataRetriever.java
@@ -2,7 +2,7 @@
  *
  * Pentaho Data Integration
  *
- * Copyright (C) 2002-2013 by Pentaho : http://www.pentaho.com
+ * Copyright (C) 2002-2016 by Pentaho : http://www.pentaho.com
  *
  *******************************************************************************
  *
@@ -35,8 +35,11 @@
 import org.pentaho.di.core.logging.LogChannelInterface;
 import org.pentaho.di.core.row.RowDataUtil;
 import org.pentaho.di.core.row.ValueMetaInterface;
+import org.pentaho.di.core.xml.XMLParserFactoryProducer;
 import org.xml.sax.Attributes;
 import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
 import org.xml.sax.helpers.DefaultHandler;
 
 /**
@@ -135,7 +138,12 @@ public void runExample() {
 
   private void parseDocument() {
     // get a factory
-    SAXParserFactory spf = SAXParserFactory.newInstance();
+    SAXParserFactory spf = null;
+    try {
+      spf = XMLParserFactoryProducer.createSecureSAXParserFactory();
+    } catch ( SAXNotSupportedException | SAXNotRecognizedException | ParserConfigurationException ex ) {
+      log.logError( ex.getMessage() );
+    }
     try {
       // get a new instance of parser
       SAXParser sp = spf.newSAXParser();

diff --git a/...ml-input-stream/src/org/pentaho/di/trans/steps/xmlinputsax/XMLInputSaxFieldRetriever.java b/...ml-input-stream/src/org/pentaho/di/trans/steps/xmlinputsax/XMLInputSaxFieldRetriever.java
@@ -2,7 +2,7 @@
  *
  * Pentaho Data Integration
  *
- * Copyright (C) 2002-2013 by Pentaho : http://www.pentaho.com
+ * Copyright (C) 2002-2016 by Pentaho : http://www.pentaho.com
  *
  *******************************************************************************
  *
@@ -33,8 +33,11 @@
 import org.pentaho.di.core.Const;
 import org.pentaho.di.core.exception.KettleValueException;
 import org.pentaho.di.core.logging.LogChannelInterface;
+import org.pentaho.di.core.xml.XMLParserFactoryProducer;
 import org.xml.sax.Attributes;
 import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
 import org.xml.sax.helpers.DefaultHandler;
 
 /**
@@ -90,7 +93,12 @@ public List<XMLInputSaxField> getFields() {
 
   private void parseDocument() {
     // get a factory
-    SAXParserFactory spf = SAXParserFactory.newInstance();
+    SAXParserFactory spf = null;
+    try {
+      spf = XMLParserFactoryProducer.createSecureSAXParserFactory();
+    } catch ( SAXNotSupportedException | SAXNotRecognizedException | ParserConfigurationException ex ) {
+      log.logError( ex.getMessage() );
+    }
     try {
       // get a new instance of parser
       SAXParser sp = spf.newSAXParser();