Fix #1859 : Sonar Vulnerability issues

apache · Dec 5, 2022 · 562dc41 · 562dc41
1 parent a79da67
commit 562dc41
Show file tree

Hide file tree

Showing 16 changed files with 138 additions and 93 deletions.
diff --git a/core/src/main/java/org/apache/hop/core/svg/HopSvgGraphics2D.java b/core/src/main/java/org/apache/hop/core/svg/HopSvgGraphics2D.java
@@ -22,6 +22,7 @@
 import org.apache.batik.svggen.DOMGroupManager;
 import org.apache.batik.svggen.SVGGraphics2D;
 import org.apache.batik.util.SVGConstants;
+import org.apache.hop.core.xml.XmlHandler;
 import org.w3c.dom.DOMImplementation;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -83,7 +84,7 @@ public static HopSvgGraphics2D newDocument() {
   }
 
   public String toXml() throws TransformerException {
-    Transformer transformer = TransformerFactory.newInstance().newTransformer();
+    Transformer transformer = XmlHandler.createSecureTransformerFactory().newTransformer();
     transformer.setOutputProperty(OutputKeys.INDENT, "yes");
     transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2");
     StreamResult streamResult = new StreamResult(new StringWriter());

diff --git a/core/src/main/java/org/apache/hop/core/svg/SvgImage.java b/core/src/main/java/org/apache/hop/core/svg/SvgImage.java
@@ -18,6 +18,7 @@
 package org.apache.hop.core.svg;
 
 import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.xml.XmlHandler;
 import org.w3c.dom.Document;
 
 import javax.xml.XMLConstants;
@@ -48,9 +49,7 @@ public static final String getSvgXml(Document document) throws HopException {
       DOMSource domSource = new DOMSource(document);
       StringWriter stringWriter = new StringWriter();
       StreamResult streamResult = new StreamResult(stringWriter);
-      TransformerFactory transformerFactory = TransformerFactory.newInstance();
-      transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
-      transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+      TransformerFactory transformerFactory = XmlHandler.createSecureTransformerFactory();
       Transformer transformer = transformerFactory.newTransformer();
       transformer.transform(domSource, streamResult);
       return stringWriter.toString();

diff --git a/core/src/main/java/org/apache/hop/core/xml/XmlHandler.java b/core/src/main/java/org/apache/hop/core/xml/XmlHandler.java
@@ -37,6 +37,7 @@
 import org.xml.sax.EntityResolver;
 import org.xml.sax.InputSource;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -732,9 +733,17 @@ public static DocumentBuilder createDocumentBuilder(
     }
   }
 
+  public static TransformerFactory createSecureTransformerFactory() {
+    TransformerFactory transformerFactory = TransformerFactory.newInstance();
+    transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+    transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+    return transformerFactory;
+  }
+
   public static String getXmlString(Node document, boolean omitXmlDeclaration, boolean indent)
       throws HopException {
-    TransformerFactory transformerFactory = TransformerFactory.newInstance();
+    TransformerFactory transformerFactory = createSecureTransformerFactory();
+
     Transformer transformer;
     try {
       transformer = transformerFactory.newTransformer();
@@ -1183,7 +1192,7 @@ public static StringBuilder closeTag(StringBuilder builder, String tag) {
   public static String formatNode(Node node) throws HopXmlException {
     StringWriter sw = new StringWriter();
     try {
-      Transformer t = TransformerFactory.newInstance().newTransformer();
+      Transformer t = createSecureTransformerFactory().newTransformer();
       t.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes");
       t.transform(new DOMSource(node), new StreamResult(sw));
     } catch (Exception e) {

diff --git a/core/src/main/java/org/apache/hop/core/xml/XmlParserFactoryProducer.java b/core/src/main/java/org/apache/hop/core/xml/XmlParserFactoryProducer.java
@@ -17,8 +17,6 @@
 
 package org.apache.hop.core.xml;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.xml.sax.SAXNotRecognizedException;
 import org.xml.sax.SAXNotSupportedException;
 
@@ -28,17 +26,14 @@
 import javax.xml.parsers.SAXParserFactory;
 
 public class XmlParserFactoryProducer {
-
-  private static final Log logger = LogFactory.getLog(XmlParserFactoryProducer.class);
-
   private XmlParserFactoryProducer() {
     // Static 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) See PPP-3506 for more details.
+   * XMLConstants#FEATURE_SECURE_PROCESSING} property. Enabling this feature protects us from some
+   * XXE attacks (e.g. XML bomb).
    *
    * @throws ParserConfigurationException if feature can't be enabled
    */
@@ -66,7 +61,7 @@ public static DocumentBuilderFactory createSecureDocBuilderFactory()
   public static SAXParserFactory createSecureSAXParserFactory()
       throws SAXNotSupportedException, SAXNotRecognizedException, ParserConfigurationException {
     SAXParserFactory factory = SAXParserFactory.newInstance();
-    factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);    
+    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);

diff --git a/engine/src/main/java/org/apache/hop/execution/remote/RemoteExecutionInfoLocation.java b/engine/src/main/java/org/apache/hop/execution/remote/RemoteExecutionInfoLocation.java
@@ -19,7 +19,6 @@
 package org.apache.hop.execution.remote;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.commons.lang.StringUtils;
 import org.apache.hop.core.exception.HopException;
 import org.apache.hop.core.gui.plugin.GuiElementType;
@@ -221,7 +220,7 @@ public List<String> getExecutionIds(boolean includeChildren, int limit) throws H
       URI uri =
           new URIBuilder(GetExecutionInfoServlet.CONTEXT_PATH)
               .addParameter(
-                  GetExecutionInfoServlet.PARAMETER_TYPE, GetExecutionInfoServlet.Type.ids.name())
+                  GetExecutionInfoServlet.PARAMETER_TYPE, GetExecutionInfoServlet.Type.IDS.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_LOCATION, location.getName())
               .addParameter(GetExecutionInfoServlet.PARAMETER_CHILDREN, includeChildren ? "Y" : "N")
               .addParameter(GetExecutionInfoServlet.PARAMETER_LIMIT, Integer.toString(limit))
@@ -243,7 +242,7 @@ public Execution getExecution(String executionId) throws HopException {
           new URIBuilder(GetExecutionInfoServlet.CONTEXT_PATH)
               .addParameter(
                   GetExecutionInfoServlet.PARAMETER_TYPE,
-                  GetExecutionInfoServlet.Type.execution.name())
+                  GetExecutionInfoServlet.Type.EXECUTION.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_LOCATION, location.getName())
               .addParameter(GetExecutionInfoServlet.PARAMETER_ID, executionId)
               .build();
@@ -262,7 +261,7 @@ public ExecutionState getExecutionState(String executionId) throws HopException
       URI uri =
           new URIBuilder(GetExecutionInfoServlet.CONTEXT_PATH)
               .addParameter(
-                  GetExecutionInfoServlet.PARAMETER_TYPE, GetExecutionInfoServlet.Type.state.name())
+                  GetExecutionInfoServlet.PARAMETER_TYPE, GetExecutionInfoServlet.Type.STATE.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_LOCATION, location.getName())
               .addParameter(GetExecutionInfoServlet.PARAMETER_ID, executionId)
               .build();
@@ -282,7 +281,7 @@ public List<Execution> findExecutions(String parentExecutionId) throws HopExcept
           new URIBuilder(GetExecutionInfoServlet.CONTEXT_PATH)
               .addParameter(
                   GetExecutionInfoServlet.PARAMETER_TYPE,
-                  GetExecutionInfoServlet.Type.children.name())
+                  GetExecutionInfoServlet.Type.CHILDREN.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_LOCATION, location.getName())
               .addParameter(GetExecutionInfoServlet.PARAMETER_ID, parentExecutionId)
               .build();
@@ -338,7 +337,7 @@ public ExecutionData getExecutionData(String parentExecutionId, String execution
       URI uri =
           new URIBuilder(GetExecutionInfoServlet.CONTEXT_PATH)
               .addParameter(
-                  GetExecutionInfoServlet.PARAMETER_TYPE, GetExecutionInfoServlet.Type.data.name())
+                  GetExecutionInfoServlet.PARAMETER_TYPE, GetExecutionInfoServlet.Type.DATA.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_LOCATION, location.getName())
               .addParameter(GetExecutionInfoServlet.PARAMETER_PARENT_ID, parentExecutionId)
               .addParameter(GetExecutionInfoServlet.PARAMETER_ID, executionId)
@@ -359,7 +358,7 @@ public Execution findLastExecution(ExecutionType executionType, String name) thr
           new URIBuilder(GetExecutionInfoServlet.CONTEXT_PATH)
               .addParameter(
                   GetExecutionInfoServlet.PARAMETER_TYPE,
-                  GetExecutionInfoServlet.Type.lastExecution.name())
+                  GetExecutionInfoServlet.Type.LAST_EXECUTION.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_LOCATION, location.getName())
               .addParameter(GetExecutionInfoServlet.PARAMETER_EXEC_TYPE, executionType.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_NAME, name)
@@ -381,7 +380,7 @@ public List<String> findChildIds(ExecutionType parentExecutionType, String paren
           new URIBuilder(GetExecutionInfoServlet.CONTEXT_PATH)
               .addParameter(
                   GetExecutionInfoServlet.PARAMETER_TYPE,
-                  GetExecutionInfoServlet.Type.childIds.name())
+                  GetExecutionInfoServlet.Type.CHILD_IDS.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_LOCATION, location.getName())
               .addParameter(GetExecutionInfoServlet.PARAMETER_EXEC_TYPE, parentExecutionType.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_ID, parentExecutionId)
@@ -403,7 +402,7 @@ public String findParentId(String childId) throws HopException {
           new URIBuilder(GetExecutionInfoServlet.CONTEXT_PATH)
               .addParameter(
                   GetExecutionInfoServlet.PARAMETER_TYPE,
-                  GetExecutionInfoServlet.Type.parentId.name())
+                  GetExecutionInfoServlet.Type.PARENT_ID.name())
               .addParameter(GetExecutionInfoServlet.PARAMETER_LOCATION, location.getName())
               .addParameter(GetExecutionInfoServlet.PARAMETER_ID, childId)
               .build();