Sonar Fixes - fix/annotate type "vulnerability" / severity "blocker"

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1870657 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/java/org/apache/poi/hssf/record/RecordFactoryInputStream.java b/src/java/org/apache/poi/hssf/record/RecordFactoryInputStream.java
index 3a3a4e33b467b670858a0a07b4173a8233807940..d7882c600f6b1e055f0e229b37adc5c8df71c26b 100644 (file)
--- a/src/java/org/apache/poi/hssf/record/RecordFactoryInputStream.java
+++ b/src/java/org/apache/poi/hssf/record/RecordFactoryInputStream.java
@@ -102,6 +102,7 @@ public final class RecordFactoryInputStream {
                        _lastRecord = rec;
                }
 
+               @SuppressWarnings({"squid:S2068"})
                public RecordInputStream createDecryptingStream(InputStream original) {
             String userPassword = Biff8EncryptionKey.getCurrentUserPassword();
                        if (userPassword == null) {

diff --git a/src/java/org/apache/poi/poifs/crypt/CryptoFunctions.java b/src/java/org/apache/poi/poifs/crypt/CryptoFunctions.java
index d4908fcae5489e6c60e36dec81d5388b76cea7a5..28e2bd75373ba388b772274ac7c2a42350793702 100644 (file)
--- a/src/java/org/apache/poi/poifs/crypt/CryptoFunctions.java
+++ b/src/java/org/apache/poi/poifs/crypt/CryptoFunctions.java
@@ -101,6 +101,7 @@ public class CryptoFunctions {
      *        if false the n-1 hash value is applied first
      * @return the hashed password
      */
+    @SuppressWarnings({"squid:S2068"})
     public static byte[] hashPassword(String password, HashAlgorithm hashAlgorithm, byte[] salt, int spinCount, boolean iteratorFirst) {
         // If no password was given, use the default
         if (password == null) {

diff --git a/src/java/org/apache/poi/poifs/crypt/Decryptor.java b/src/java/org/apache/poi/poifs/crypt/Decryptor.java
index 07c5666e6b67b3c545cc7ee8c28c028bf9de6b9f..ad75f5b134f7e136b9be29315ea087dfd453a859 100644 (file)
--- a/src/java/org/apache/poi/poifs/crypt/Decryptor.java
+++ b/src/java/org/apache/poi/poifs/crypt/Decryptor.java
@@ -33,6 +33,7 @@ import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.util.GenericRecordUtil;
 
 public abstract class Decryptor implements Cloneable, GenericRecord {
+    @SuppressWarnings({"squid:S2068"})
     public static final String DEFAULT_PASSWORD="VelvetSweatshop";
     public static final String DEFAULT_POIFS_ENTRY="EncryptedPackage";
     

diff --git a/src/java/org/apache/poi/util/StaxHelper.java b/src/java/org/apache/poi/util/StaxHelper.java
index 31530c2e7b58600ac0bbdd4e986128f8a6bf6a50..1ce05b9eff097fe17f323737288e6658797d8a7c 100644 (file)
--- a/src/java/org/apache/poi/util/StaxHelper.java
+++ b/src/java/org/apache/poi/util/StaxHelper.java
@@ -17,6 +17,8 @@
 
 package org.apache.poi.util;
 
+import java.util.function.Consumer;
+
 import javax.xml.stream.XMLEventFactory;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLOutputFactory;
@@ -28,17 +30,19 @@ import javax.xml.stream.XMLOutputFactory;
 public final class StaxHelper {
     private static final POILogger logger = POILogFactory.getLogger(StaxHelper.class);
 
-    private StaxHelper() {}
+    private StaxHelper() {
+    }
 
     /**
      * Creates a new StAX XMLInputFactory, with sensible defaults
      */
+    @SuppressWarnings({"squid:S2755"})
     public static XMLInputFactory newXMLInputFactory() {
         XMLInputFactory factory = XMLInputFactory.newInstance();
-        trySetProperty(factory, XMLInputFactory.IS_NAMESPACE_AWARE, true);
-        trySetProperty(factory, XMLInputFactory.IS_VALIDATING, false);
-        trySetProperty(factory, XMLInputFactory.SUPPORT_DTD, false);
-        trySetProperty(factory, XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+        trySet(XMLInputFactory.IS_NAMESPACE_AWARE, (n) -> factory.setProperty(n, true));
+        trySet(XMLInputFactory.IS_VALIDATING, (n) -> factory.setProperty(n, false));
+        trySet(XMLInputFactory.SUPPORT_DTD, (n) -> factory.setProperty(n, false));
+        trySet(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, (n) -> factory.setProperty(n, false));
         return factory;
     }
 
@@ -47,7 +51,7 @@ public final class StaxHelper {
      */
     public static XMLOutputFactory newXMLOutputFactory() {
         XMLOutputFactory factory = XMLOutputFactory.newInstance();
-        trySetProperty(factory, XMLOutputFactory.IS_REPAIRING_NAMESPACES, true);
+        trySet(XMLOutputFactory.IS_REPAIRING_NAMESPACES, (n) -> factory.setProperty(n, true));
         return factory;
     }
 
@@ -58,24 +62,14 @@ public final class StaxHelper {
         // this method seems safer on Android than getFactory()
         return XMLEventFactory.newInstance();
     }
-            
-    private static void trySetProperty(XMLInputFactory factory, String feature, boolean flag) {
-        try {
-            factory.setProperty(feature, flag);
-        } catch (Exception e) {
-            logger.log(POILogger.WARN, "StAX Property unsupported", feature, e);
-        } catch (AbstractMethodError ame) {
-            logger.log(POILogger.WARN, "Cannot set StAX property because outdated StAX parser in classpath", feature, ame);
-        }
-    }
 
-    private static void trySetProperty(XMLOutputFactory factory, String feature, boolean flag) {
+    private static void trySet(String name, Consumer<String> securityFeature) {
         try {
-            factory.setProperty(feature, flag);
+            securityFeature.accept(name);
         } catch (Exception e) {
-            logger.log(POILogger.WARN, "StAX Property unsupported", feature, e);
+            logger.log(POILogger.WARN, "StAX Property unsupported", name, e);
         } catch (AbstractMethodError ame) {
-            logger.log(POILogger.WARN, "Cannot set StAX property because outdated StAX parser in classpath", feature, ame);
+            logger.log(POILogger.WARN, "Cannot set StAX property because outdated StAX parser in classpath", name, ame);
         }
     }
 }

diff --git a/src/java/org/apache/poi/util/XMLHelper.java b/src/java/org/apache/poi/util/XMLHelper.java
index 8797475461e0839179cc0227a398eca1bda22dce..9c29546eba8e0e37c3bf0f3ea3f2038a516dd79f 100644 (file)
--- a/src/java/org/apache/poi/util/XMLHelper.java
+++ b/src/java/org/apache/poi/util/XMLHelper.java
@@ -19,37 +19,47 @@ package org.apache.poi.util;
 
 import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
 
 /**
  * Helper methods for working with javax.xml classes.
  */
-public final class XMLHelper
-{
+public final class XMLHelper {
     private static POILogger logger = POILogFactory.getLogger(XMLHelper.class);
-    
+
+    @FunctionalInterface
+    private interface SecurityFeature {
+        void accept(String name) throws ParserConfigurationException;
+    }
+
     /**
      * Creates a new DocumentBuilderFactory, with sensible defaults
+     *
+     * @see <a href="https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html">OWASP XXE</a>
      */
+    @SuppressWarnings({"squid:S2755"})
     public static DocumentBuilderFactory getDocumentBuilderFactory() {
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
         factory.setExpandEntityReferences(false);
-        trySetSAXFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true);
-        trySetSAXFeature(factory, "http://xml.org/sax/features/external-general-entities", false);
-        trySetSAXFeature(factory, "http://xml.org/sax/features/external-parameter-entities", false);
-        trySetSAXFeature(factory, "http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
-        trySetSAXFeature(factory, "http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
+        trySet(XMLConstants.FEATURE_SECURE_PROCESSING, (n) -> factory.setFeature(n, true));
+        trySet(XMLConstants.ACCESS_EXTERNAL_SCHEMA, (n) -> factory.setAttribute(n, ""));
+        trySet(XMLConstants.ACCESS_EXTERNAL_DTD, (n) -> factory.setAttribute(n, ""));
+        trySet("http://xml.org/sax/features/external-general-entities", (n) -> factory.setFeature(n, false));
+        trySet("http://xml.org/sax/features/external-parameter-entities", (n) -> factory.setFeature(n, false));
+        trySet("http://apache.org/xml/features/nonvalidating/load-external-dtd", (n) -> factory.setFeature(n, false));
+        trySet("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", (n) -> factory.setFeature(n, false));
+        trySet("http://apache.org/xml/features/disallow-doctype-decl", (n) -> factory.setFeature(n, true));
+        trySet("XIncludeAware", (n) -> factory.setXIncludeAware(false));
         return factory;
     }
-    
-    private static void trySetSAXFeature(DocumentBuilderFactory documentBuilderFactory, String feature, boolean enabled) {
+
+    private static void trySet(String name, SecurityFeature feature) {
         try {
-            documentBuilderFactory.setFeature(feature, enabled);
+            feature.accept(name);
         } catch (Exception e) {
-            logger.log(POILogger.WARN, "SAX Feature unsupported", feature, e);
+            logger.log(POILogger.WARN, "SAX Feature unsupported", name, e);
         } catch (AbstractMethodError ame) {
-            logger.log(POILogger.WARN, "Cannot set SAX feature because outdated XML parser in classpath", feature, ame);
+            logger.log(POILogger.WARN, "Cannot set SAX feature because outdated XML parser in classpath", name, ame);
         }
     }
-    
-
 }

diff --git a/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExportToXml.java b/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExportToXml.java
index 65cb5a981c61630e797e092a3273296d37891924..e870349b4db3a9c369bbaf573234927d3ef8a2e9 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExportToXml.java
+++ b/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExportToXml.java
@@ -39,10 +39,10 @@ import javax.xml.validation.Schema;
 import javax.xml.validation.SchemaFactory;
 import javax.xml.validation.Validator;
 
+import org.apache.poi.ooxml.util.DocumentHelper;
 import org.apache.poi.ooxml.util.TransformerHelper;
 import org.apache.poi.ss.usermodel.CellType;
 import org.apache.poi.ss.usermodel.DateUtil;
-import org.apache.poi.ooxml.util.DocumentHelper;
 import org.apache.poi.util.LocaleUtil;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
@@ -82,6 +82,13 @@ import org.xml.sax.SAXException;
 public class XSSFExportToXml implements Comparator<String>{
     private static final POILogger LOG = POILogFactory.getLogger(XSSFExportToXml.class);
 
+
+    @FunctionalInterface
+    private interface SecurityFeature {
+        void accept(String name) throws SAXException;
+    }
+
+
     private XSSFMap map;
     private final HashMap<String, Integer> indexMap = new HashMap<>();
     /**
@@ -240,11 +247,13 @@ public class XSSFExportToXml implements Comparator<String>{
      * @return true, if document is valid
      * @throws SAXException If validating the document fails
      */
+    @SuppressWarnings({"squid:S2755"})
     private boolean isValid(Document xml) throws SAXException{
         try {
-            String language = "http://www.w3.org/2001/XMLSchema";
-            SchemaFactory factory = SchemaFactory.newInstance(language);
-            trySetFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
+            trySet(XMLConstants.FEATURE_SECURE_PROCESSING, (n) -> factory.setFeature(n, true));
+            trySet(XMLConstants.ACCESS_EXTERNAL_DTD, (n) -> factory.setProperty(n,""));
+            trySet(XMLConstants.ACCESS_EXTERNAL_SCHEMA, (n) -> factory.setProperty(n,""));
 
             Source source = new DOMSource(map.getSchema());
             Schema schema = factory.newSchema(source);
@@ -537,13 +546,13 @@ public class XSSFExportToXml implements Comparator<String>{
         return complexTypeNode;
     }
 
-    private static void trySetFeature(SchemaFactory sf, String feature, boolean enabled) {
+    private static void trySet(String name, SecurityFeature securityFeature) {
         try {
-            sf.setFeature(feature, enabled);
+            securityFeature.accept(name);
         } catch (Exception e) {
-            LOG.log(POILogger.WARN, "SchemaFactory Feature unsupported", feature, e);
+            LOG.log(POILogger.WARN, "SchemaFactory feature unsupported", name, e);
         } catch (AbstractMethodError ame) {
-            LOG.log(POILogger.WARN, "Cannot set SchemaFactory feature because outdated XML parser in classpath", feature, ame);
+            LOG.log(POILogger.WARN, "Cannot set SchemaFactory feature because outdated XML parser in classpath", name, ame);
         }
     }
 }