From: Andreas Beeker Date: Sun, 1 Dec 2019 02:05:51 +0000 (+0000) Subject: Sonar Fixes - fix/annotate type "vulnerability" / severity "blocker" X-Git-Tag: REL_4_1_2~113 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=a17a9168b112a1d7df99d3aa4bc054528c680133;p=poi.git 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 3a3a4e33b4..d7882c600f 100644 --- 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 d4908fcae5..28e2bd7537 100644 --- 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 07c5666e6b..ad75f5b134 100644 --- 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 31530c2e7b..1ce05b9eff 100644 --- 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 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 8797475461..9c29546eba 100644 --- 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 OWASP XXE */ + @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 65cb5a981c..e870349b4d 100644 --- 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{ 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 indexMap = new HashMap<>(); /** @@ -240,11 +247,13 @@ public class XSSFExportToXml implements Comparator{ * @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{ 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); } } }