Merge "Omaha #4491: Address security scan findings in IscReceiveSrv." into omaha_16.1.1

Former-commit-id: 51ecf21dc606759e9e70d2d67160a69517ccbcab

edexOsgi/com.raytheon.edex.plugin.gfe/src/com/raytheon/edex/plugin/gfe/isc/IscReceiveSrv.java

@@ -19,15 +19,17 @@
  **/
 package com.raytheon.edex.plugin.gfe.isc;
 
-import java.io.File;
-import java.io.FilenameFilter;
 import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -51,8 +53,6 @@ import com.raytheon.uf.common.python.concurrent.PythonJobCoordinator;
 import com.raytheon.uf.common.status.IUFStatusHandler;
 import com.raytheon.uf.common.status.UFStatus;
 import com.raytheon.uf.common.status.UFStatus.Priority;
-import com.raytheon.uf.common.util.FileUtil;
-import com.raytheon.uf.common.util.file.FilenameFilters;
 
 /**
  * ISC data receive service. Takes incoming request and executes iscDataRec
@@ -69,6 +69,7 @@ import com.raytheon.uf.common.util.file.FilenameFilters;
  * Mar 14, 2013   #1794    djohnson    Consolidate common FilenameFilter implementations.
  * Dec 10, 2014   #4953    randerso    Properly handle single file reception
  * May 06, 2015   #4383    dgilling    Properly XML parse incoming XML file.
+ * May 20, 2015   #4491    dgilling    Remediate path manipulation possibilities.
  * 
  * </pre>
  * 
@@ -81,12 +82,23 @@ public class IscReceiveSrv {
     private static final transient IUFStatusHandler statusHandler = UFStatus
             .getHandler(IscReceiveSrv.class);
 
-    private static final String ISC_REQUEST = "iscrequest";
-
     private static final String METHOD_NAME = "main";
 
-    private static final FilenameFilter docFileFilter = FilenameFilters
+    /*
-            .byFileExtension(".doc");
+     * TODO: determine if this constant and the cleanup of DOC files in the
+     * finally block of prepareIscDataRec() is still necessary.
+     */
+    private static final String DOC_FILE_FILTER = "*.doc";
+
+    private static final Path ISC_PRODUCTS_DIR = Paths.get("/awips2",
+            "GFESuite", "products", "ISC");
+
+    private static final Collection<Path> WHITELISTED_PATHS = Arrays
+            .asList(ISC_PRODUCTS_DIR);
+
+    private static final String COPY_ERROR_MSG = "Failed to copy: [%s] to %s. Unable to execute iscDataRec for %s.";
+
+    private static final String UNSAFE_PATH_MSG = "Skipping iscDataRec processing because file %s comes from an unsafe location.";
 
     private static final IPythonJobListener<String> jobListener = new IPythonJobListener<String>() {
 
@@ -150,131 +162,102 @@ public class IscReceiveSrv {
             throws IOException, InterruptedException,
             GfeConfigurationException, SAXException,
             ParserConfigurationException {
-        Map<String, String[]> siteMap = new HashMap<String, String[]>();
+        Map<String, String[]> siteMap = new HashMap<>();
 
         String[] incomingFiles = args[2].split(",");
-        String xmlFileName = "";
+        String xmlPathString = (incomingFiles.length == 1) ? incomingFiles[0]
-        String dataFileName = null;
+                : incomingFiles[1];
-        if (incomingFiles.length == 1) {
+        String dataPathString = (incomingFiles.length == 1) ? null
-            xmlFileName = incomingFiles[0];
+                : incomingFiles[0];
-        } else {
+        Path xmlFilePath = Paths.get(xmlPathString);
-            dataFileName = incomingFiles[0];
+        Path dataFilePath = (dataPathString != null) ? Paths
-            xmlFileName = incomingFiles[1];
+                .get(dataPathString) : null;
-        }
 
-        final File incomingXMLFile = new File(xmlFileName);
-        DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
-        DocumentBuilder db = dbf.newDocumentBuilder();
-        Document doc = db.parse(incomingXMLFile);
-        doc.getDocumentElement().normalize();
-
-        Collection<String> siteList = getXMLDestinations(doc);
-        Set<String> activeSites = IFPServer.getActiveSites();
-
-        if (ISC_REQUEST.equals(doc.getDocumentElement().getNodeName())) {
-            /*
-             * This case is for processing an ISC Request/Reply message
-             * requesting our site's grids.
-             */
-            for (String siteId : siteList) {
-                if (activeSites.contains(siteId)) {
-                    if (IFPServerConfigManager.getServerConfig(siteId)
-                            .requestISC()) {
-                        String[] newArgs = new String[args.length];
-                        System.arraycopy(args, 0, newArgs, 0, args.length);
-                        String newXmlFileName = xmlFileName + "." + siteId;
-                        FileUtil.copyFile(incomingXMLFile, new File(
-                                newXmlFileName));
-                        newArgs[2] = newXmlFileName;
-                        siteMap.put(siteId, newArgs);
-                    }
-                }
-            }
-            incomingXMLFile.delete();
-        } else {
         try {
-                for (String site : siteList) {
+            if (!isSafePathToProcess(xmlFilePath)) {
-                    if (activeSites.contains(site)
+                statusHandler.warn(String.format(UNSAFE_PATH_MSG, xmlFilePath));
-                            && IFPServerConfigManager.getServerConfig(site)
+                return Collections.emptyMap();
-                                    .requestISC()) {
+            }
+
+            if ((dataFilePath != null) && (!isSafePathToProcess(dataFilePath))) {
+                statusHandler
+                        .warn(String.format(UNSAFE_PATH_MSG, dataFilePath));
+                return Collections.emptyMap();
+            }
+
+            String xmlFileName = xmlFilePath.getFileName().toString();
+            String dataFileName = (dataPathString != null) ? dataFilePath
+                    .getFileName().toString() : null;
+
+            Collection<String> destinations = getXMLDestinations(xmlFilePath);
+            Set<String> activeSites = IFPServer.getActiveSites();
+            Set<String> activeDestinations = new HashSet<>(activeSites);
+            activeDestinations.retainAll(destinations);
+
+            for (String siteId : activeDestinations) {
+                if (IFPServerConfigManager.getServerConfig(siteId).requestISC()) {
                     String[] modifiedArgs = new String[args.length];
                     System.arraycopy(args, 0, modifiedArgs, 0, args.length);
 
-                        if (dataFileName != null) {
+                    if (dataFilePath != null) {
-                            String newFileName = dataFileName + "." + site;
+                        String newDataFileName = dataFileName + "." + siteId;
+                        Path newDataFilePath = dataFilePath
+                                .resolveSibling(newDataFileName);
+
                         try {
-                                FileUtil.copyFile(new File(dataFileName),
+                            Files.copy(dataFilePath, newDataFilePath,
-                                        new File(newFileName));
+                                    StandardCopyOption.REPLACE_EXISTING);
                         } catch (IOException e) {
-                                statusHandler
+                            statusHandler.error(String.format(COPY_ERROR_MSG,
-                                        .error("Failed to copy: ["
+                                    dataFilePath, newDataFilePath, siteId), e);
-                                                + dataFileName
-                                                + "] to "
-                                                + newFileName
-                                                + ".  Unable to execute iscDataRec for "
-                                                + site, e);
                             continue;
                         }
 
-                            if (!new File(newFileName).exists()) {
-                                statusHandler
-                                        .error("Failed to copy: ["
-                                                + dataFileName
-                                                + "] to "
-                                                + newFileName
-                                                + ".  Unable to execute iscDataRec for "
-                                                + site);
-                                continue;
-                            }
                         modifiedArgs[2] = modifiedArgs[2].replace(
-                                    dataFileName, newFileName);
+                                dataPathString, newDataFilePath.toString());
                     }
 
-                        String newXmlFileName = xmlFileName + "." + site;
+                    String newXmlFileName = xmlFileName + "." + siteId;
+                    Path newXmlFilePath = xmlFilePath
+                            .resolveSibling(newXmlFileName);
+
                     try {
-                            FileUtil.copyFile(new File(xmlFileName), new File(
+                        Files.copy(xmlFilePath, newXmlFilePath,
-                                    newXmlFileName));
+                                StandardCopyOption.REPLACE_EXISTING);
                     } catch (IOException e) {
-                            statusHandler.error("Failed to copy: ["
+                        statusHandler.error(String.format(COPY_ERROR_MSG,
-                                    + xmlFileName + "] to " + newXmlFileName
+                                xmlFilePath, newXmlFilePath, siteId), e);
-                                    + ".  Unable to execute iscDataRec for "
-                                    + site, e);
-                            continue;
-                        }
-                        if (!new File(newXmlFileName).exists()) {
-                            statusHandler.error("Failed to copy: ["
-                                    + xmlFileName + "] to " + newXmlFileName
-                                    + ".  Unable to execute iscDataRec for "
-                                    + site);
                         continue;
                     }
 
-                        modifiedArgs[2] = modifiedArgs[2].replace(xmlFileName,
+                    modifiedArgs[2] = modifiedArgs[2].replace(xmlPathString,
-                                newXmlFileName);
+                            newXmlFilePath.toString());
-                        siteMap.put(site, modifiedArgs);
+
+                    siteMap.put(siteId, modifiedArgs);
                 }
             }
         } finally {
-                if (dataFileName != null) {
+            Collection<Path> filesToDelete = new HashSet<>();
-                    File dataFile = new File(dataFileName);
+
-                    if (dataFile.exists()) {
+            filesToDelete.add(xmlFilePath);
-                        if (!dataFile.delete()) {
+            if (dataFilePath != null) {
-                            statusHandler.error("Unable to delete "
+                filesToDelete.add(dataFilePath);
-                                    + dataFileName);
             }
+            try (DirectoryStream<Path> stream = Files.newDirectoryStream(
+                    xmlFilePath.getParent(), DOC_FILE_FILTER)) {
+                for (Path entry : stream) {
+                    filesToDelete.add(entry);
                 }
+            } catch (IOException e) {
+                statusHandler.error("Unable to list .doc files in directory "
+                        + xmlFilePath.getParent(), e);
             }
 
-                File xmlFile = incomingXMLFile;
+            for (Path toDelete : filesToDelete) {
-                if (xmlFile.exists()) {
+                try {
-                    if (!xmlFile.delete()) {
+                    Files.deleteIfExists(toDelete);
-                        statusHandler.error("Unable to delete " + xmlFileName);
+                } catch (IOException e) {
-                    }
+                    statusHandler.error("Unable to delete file " + toDelete, e);
-                }
-                List<File> docFiles = FileUtil.listFiles(
-                        xmlFile.getParentFile(), docFileFilter, false);
-                for (File docFile : docFiles) {
-                    docFile.delete();
                 }
             }
         }
@@ -282,9 +265,29 @@ public class IscReceiveSrv {
         return siteMap;
     }
 
-    private Collection<String> getXMLDestinations(final Document doc)
+    private boolean isSafePathToProcess(Path path) {
+        try {
+            Path realPath = path.toRealPath();
+            for (Path safePath : WHITELISTED_PATHS) {
+                if (realPath.startsWith(safePath)) {
+                    return true;
+                }
+            }
+        } catch (IOException e) {
+            statusHandler.error("Unable to resolve the real path for " + path,
+                    e);
+        }
+
+        return false;
+    }
+
+    private Collection<String> getXMLDestinations(final Path xmlDocumentPath)
             throws SAXException, IOException, ParserConfigurationException {
-        Collection<String> destinations = new HashSet<>();
+        DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+        dbf.setExpandEntityReferences(false);
+        DocumentBuilder db = dbf.newDocumentBuilder();
+        Document doc = db.parse(xmlDocumentPath.toFile());
+        doc.getDocumentElement().normalize();
 
         // Expected XML format:
         // <isc>
@@ -295,6 +298,7 @@ public class IscReceiveSrv {
         // </address>
         // </destinations>
         // </isc>
+        Collection<String> destinations = new HashSet<>();
         NodeList destNodes = doc.getElementsByTagName("destinations");
         if (destNodes.getLength() > 0) {
             Node destNode = destNodes.item(0);