Fix zip slip security flaw

Apply zip slip checking in zip operations throughout the system.
Centralizes most of the zip logic in one class. Create tests to zip
functionalities and zip slip problem.

Change-Id: I721f3d44b34fe6d242c9537f5a515ce1bb534c9a
Issue-ID: SDC-1401
Signed-off-by: andre.schmid <andre.schmid@est.tech>

2 files changed, 42 insertions, 94 deletions

diff --git a/openecomp-be/lib/openecomp-common-lib/src/main/java/org/openecomp/sdc/common/utils/CommonUtil.java b/openecomp-be/lib/openecomp-common-lib/src/main/java/org/openecomp/sdc/common/utils/CommonUtil.java
index ae7d44efd8..8610ecb74b 100644
--- a/openecomp-be/lib/openecomp-common-lib/src/main/java/org/openecomp/sdc/common/utils/CommonUtil.java
+++ b/openecomp-be/lib/openecomp-common-lib/src/main/java/org/openecomp/sdc/common/utils/CommonUtil.java
@@ -20,62 +20,50 @@
 package org.openecomp.sdc.common.utils;
 
 import com.google.common.collect.Multimap;
-
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
 import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
 import org.openecomp.core.utilities.file.FileContentHandler;
-import org.openecomp.core.utilities.file.FileUtils;
 import org.openecomp.core.utilities.orchestration.OnboardingTypesEnum;
 import org.openecomp.sdc.common.errors.CoreException;
 import org.openecomp.sdc.common.errors.ErrorCategory;
 import org.openecomp.sdc.common.errors.ErrorCode;
 import org.openecomp.sdc.common.errors.Messages;
-
-import java.io.ByteArrayInputStream;
-import java.io.File;
-import java.io.IOException;
-import java.lang.reflect.Field;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.Set;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipException;
-import java.util.zip.ZipInputStream;
+import org.openecomp.sdc.common.zip.ZipUtils;
+import org.openecomp.sdc.common.zip.exception.ZipException;
 
 public class CommonUtil {
-  static final String DEFAULT = "default";
-  static final String _DEFAULT = "_default";
 
   private CommonUtil() {
     // prevent instantiation
   }
 
-  public static FileContentHandler validateAndUploadFileContent(OnboardingTypesEnum type,
-                                                                byte[] uploadedFileData)
-      throws IOException {
-    return getFileContentMapFromOrchestrationCandidateZipAndValidateNoFolders(type,
-        uploadedFileData);
-  }
-
   /**
-   * Gets files out of the zip AND validates zip is flat (no folders)
+   * Reads the files from the zip AND validates zip is flat (no folders).
    *
-   * @param uploadFileData zip file
+   * @param type the onboarding type
+   * @param uploadedFileData zip file bytes
    * @return FileContentHandler if input is valid and has no folders
+   * @throws IOException when the zip could not be read
    */
-  private static FileContentHandler getFileContentMapFromOrchestrationCandidateZipAndValidateNoFolders(
-      OnboardingTypesEnum type, byte[] uploadFileData)
-      throws IOException {
-    Pair<FileContentHandler, List<String>> pair =
-        getFileContentMapFromOrchestrationCandidateZip(uploadFileData);
-
+  public static FileContentHandler validateAndUploadFileContent(final OnboardingTypesEnum type,
+                                                                final byte[] uploadedFileData) throws IOException {
+    final Pair<FileContentHandler, List<String>> pair;
+    try {
+      pair = getFileContentMapFromOrchestrationCandidateZip(uploadedFileData);
+    } catch (final ZipException e) {
+      throw new IOException(e);
+    }
     if (isFileOriginFromZip(type.toString())) {
       validateNoFolders(pair.getRight());
     }
@@ -84,47 +72,22 @@ public class CommonUtil {
   }
 
   public static Pair<FileContentHandler, List<String>> getFileContentMapFromOrchestrationCandidateZip(
-      byte[] uploadFileData)
-      throws IOException {
-    ZipEntry zipEntry;
-    List<String> folderList = new ArrayList<>();
-    FileContentHandler mapFileContent = new FileContentHandler();
-    try (ByteArrayInputStream in = new ByteArrayInputStream(uploadFileData);
-         ZipInputStream inputZipStream = new ZipInputStream(in)) {
-      byte[] fileByteContent;
-      String currentEntryName;
-
-      while ((zipEntry = inputZipStream.getNextEntry()) != null) {
-        assertEntryNotVulnerable(zipEntry);
-        currentEntryName = zipEntry.getName();
-        fileByteContent = FileUtils.toByteArray(inputZipStream);
-
-        int index = lastIndexFileSeparatorIndex(currentEntryName);
-        if (index != -1) {
-          folderList.add(currentEntryName);
-        }
-        if (isFile(currentEntryName)) {
-          mapFileContent.addFile(currentEntryName, fileByteContent);
-        }
+      byte[] uploadFileData) throws ZipException {
+    final Map<String, byte[]> zipFileMap = ZipUtils.readZip(uploadFileData, true);
+    final List<String> folderList = new ArrayList<>();
+    final FileContentHandler mapFileContent = new FileContentHandler();
+
+    zipFileMap.forEach((key, value) -> {
+      if (value == null) {
+        folderList.add(key);
+      } else {
+        mapFileContent.addFile(key, value);
       }
-
-    } catch (RuntimeException exception) {
-      throw new IOException(exception);
-    }
+    });
 
     return new ImmutablePair<>(mapFileContent, folderList);
   }
 
-  private static void assertEntryNotVulnerable(ZipEntry entry) throws ZipException {
-    if (entry.getName().contains("../")) {
-      throw new ZipException("Path traversal attempt discovered.");
-    }
-  }
-
-  private static boolean isFile(String currentEntryName) {
-    return !(currentEntryName.endsWith("\\") || currentEntryName.endsWith("/"));
-  }
-
   private static void validateNoFolders(List<String> folderList) {
     if (CollectionUtils.isNotEmpty(folderList)) {
       throw new CoreException((new ErrorCode.ErrorCodeBuilder())
@@ -134,19 +97,6 @@ public class CommonUtil {
     }
   }
 
-  private static int lastIndexFileSeparatorIndex(String filePath) {
-    int length = filePath.length() - 1;
-
-    for (int i = length; i >= 0; i--) {
-      char currChar = filePath.charAt(i);
-      if (currChar == '/' || currChar == File.separatorChar || currChar == File.pathSeparatorChar) {
-        return i;
-      }
-    }
-    // if we've reached to the start of the string and didn't find file separator - return -1
-    return -1;
-  }
-
   private static boolean validateFilesExtensions(Set<String> allowedExtensions, FileContentHandler
       files) {
     for (String fileName : files.getFileList()) {
diff --git a/openecomp-be/lib/openecomp-common-lib/src/test/java/org/openecomp/sdc/common/utils/CommonUtilTest.java b/openecomp-be/lib/openecomp-common-lib/src/test/java/org/openecomp/sdc/common/utils/CommonUtilTest.java
index bc0bd137a0..119616a9be 100644
--- a/openecomp-be/lib/openecomp-common-lib/src/test/java/org/openecomp/sdc/common/utils/CommonUtilTest.java
+++ b/openecomp-be/lib/openecomp-common-lib/src/test/java/org/openecomp/sdc/common/utils/CommonUtilTest.java
@@ -16,23 +16,21 @@
 
 package org.openecomp.sdc.common.utils;
 
-import org.testng.annotations.Test;
+import static org.onap.sdc.tosca.services.CommonUtil.DEFAULT;
+import static org.onap.sdc.tosca.services.CommonUtil.UNDERSCORE_DEFAULT;
+import static org.testng.Assert.assertTrue;
 
 import java.util.HashMap;
 import java.util.Map;
-
-import static org.testng.Assert.assertTrue;
+import org.testng.annotations.Test;
 
 public class CommonUtilTest {
 
     @Test
     public void testGetObjectAsMap() {
-        Map<String, String> obj = new HashMap<>(1);
-        obj.put(CommonUtil.DEFAULT, "");
-        Map<String, Object> newMap = CommonUtil.getObjectAsMap(obj);
-
-        boolean exists = newMap.containsKey(CommonUtil._DEFAULT);
-
-        assertTrue(exists);
+        final Map<String, String> obj = new HashMap<>(1);
+        obj.put(DEFAULT, "");
+        assertTrue(CommonUtil.getObjectAsMap(obj).containsKey(UNDERSCORE_DEFAULT));
     }
+
 }