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, 58 insertions, 40 deletions

diff --git a/openecomp-be/lib/openecomp-sdc-externaltesting-lib/openecomp-sdc-externaltesting-impl/src/main/java/org/openecomp/core/externaltesting/impl/ExternalTestingManagerImpl.java b/openecomp-be/lib/openecomp-sdc-externaltesting-lib/openecomp-sdc-externaltesting-impl/src/main/java/org/openecomp/core/externaltesting/impl/ExternalTestingManagerImpl.java
index e112ef432c..22ac1e8ed8 100644
--- a/openecomp-be/lib/openecomp-sdc-externaltesting-lib/openecomp-sdc-externaltesting-impl/src/main/java/org/openecomp/core/externaltesting/impl/ExternalTestingManagerImpl.java
+++ b/openecomp-be/lib/openecomp-sdc-externaltesting-lib/openecomp-sdc-externaltesting-impl/src/main/java/org/openecomp/core/externaltesting/impl/ExternalTestingManagerImpl.java
@@ -18,17 +18,21 @@ package org.openecomp.core.externaltesting.impl;
 
 import com.amdocs.zusammen.utils.fileutils.json.JsonUtil;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableSet;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParseException;
+import java.util.Map.Entry;
 import lombok.EqualsAndHashCode;
-import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
 import org.onap.sdc.tosca.services.YamlUtil;
 import org.openecomp.core.externaltesting.api.*;
 import org.openecomp.core.externaltesting.errors.ExternalTestingException;
+import org.openecomp.sdc.common.zip.ZipUtils;
+import org.openecomp.sdc.common.zip.exception.ZipException;
 import org.openecomp.sdc.heat.datatypes.manifest.FileData;
 import org.openecomp.sdc.heat.datatypes.manifest.ManifestContent;
 import org.openecomp.sdc.vendorsoftwareproduct.OrchestrationTemplateCandidateManager;
@@ -58,7 +62,6 @@ import java.util.*;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.zip.ZipEntry;
-import java.util.zip.ZipInputStream;
 import java.util.zip.ZipOutputStream;
 
 public class ExternalTestingManagerImpl implements ExternalTestingManager {
@@ -97,6 +100,8 @@ public class ExternalTestingManagerImpl implements ExternalTestingManager {
 
   private static final String SDC_CSAR = "sdc-csar";
   private static final String SDC_HEAT = "sdc-heat";
+  private final ImmutableSet<String> relevantArchiveFileExtensionSet =
+      ImmutableSet.of("yaml", "meta", "yml", "json", "env");
 
 
   private VersioningManager versioningManager;
@@ -721,10 +726,8 @@ public class ExternalTestingManagerImpl implements ExternalTestingManager {
   private void processArchive(final VtpTestExecutionRequest test, final MultiValueMap<String, Object> body, final byte[] zip) {
 
     // We need to make one pass through the zip input stream.  Pull out files that match our expectations into a temporary
-    // map that we can process over.   These are not huge files so we shouldn't need to worry about memory.
-
-    List<String> extensions = Arrays.asList(".yaml", ".meta", ".yml", ".json", ".env");
-    final Map<String, byte[]> contentWeCareAbout = extractRelevantContent(zip, extensions);
+    // map that we can process over. These are not huge files so we shouldn't need to worry about memory.
+    final Map<String, byte[]> contentWeCareAbout = extractRelevantContent(zip);
 
     // VTP does not support concurrent executions of the same test with the same associated file name.
     // It writes files to /tmp and if we were to send two requests with the same file, the results are unpredictable.
@@ -891,34 +894,30 @@ public class ExternalTestingManagerImpl implements ExternalTestingManager {
    * @param zip csar/heat zip to iterate over
    * @return relevant content from the archive file as a map.
    */
-  private Map<String, byte[]> extractRelevantContent(final byte[] zip, final List<String> extensions) {
-    final Map<String, byte[]> rv = new HashMap<>();  // FYI, rv = return value.
-    try (ByteArrayInputStream is = new ByteArrayInputStream(zip)) {
-      try (ZipInputStream zipStream = new ZipInputStream(is)) {
-        ZipEntry entry;
-        while ((entry = zipStream.getNextEntry()) != null) {
-          final String entryName = entry.getName();
-
-          // NOTE: leaving this debugging in for dublin...
-          logger.debug("archive contains entry {}", entryName);
-
-          extractIfMatching(extensions, rv, zipStream, entryName);
-        }
-      }
-    }
-    catch (IOException ex) {
-      logger.error("error encountered processing archive", ex);
-      throw new ExternalTestingException(SDC_RESOLVER_ERR, 500, ex.getMessage());
+  private Map<String, byte[]> extractRelevantContent(final byte[] zip) {
+    final Map<String, byte[]> zipFileAndByteMap;
+    try {
+      zipFileAndByteMap = ZipUtils.readZip(zip, false);
+    } catch (final ZipException ex) {
+      logger.error("An error occurred while processing archive", ex);
+      throw new ExternalTestingException(SDC_RESOLVER_ERR, 500, ex.getMessage(), ex);
     }
-    return rv;
+
+    return zipFileAndByteMap.entrySet().stream()
+        .filter(stringEntry -> hasRelevantExtension(stringEntry.getKey()))
+        .collect(Collectors.toMap(Entry::getKey, Entry::getValue));
   }
 
-  private void extractIfMatching(List<String> extensions, Map<String, byte[]> rv, ZipInputStream zipStream, String entryName) throws IOException {
-    int idx = entryName.lastIndexOf('.');
-    if ((idx >= 0) && (extensions.contains(entryName.substring(idx)))) {
-      byte[] content = IOUtils.toByteArray(zipStream);
-      rv.put(entryName, content);
-    }
+  /**
+   * Checks if the file matches with a expected extension.
+   *
+   * @param filePath the file path
+   * @return {@code true} if the file extension matches with {@link #relevantArchiveFileExtensionSet}, {@code false}
+   * otherwise
+   */
+  private boolean hasRelevantExtension(final String filePath) {
+    final String entryExtension = FilenameUtils.getExtension(filePath);
+    return StringUtils.isNotEmpty(entryExtension) && (relevantArchiveFileExtensionSet.contains(entryExtension));
   }
 
   /**
diff --git a/openecomp-be/lib/openecomp-sdc-externaltesting-lib/openecomp-sdc-externaltesting-impl/src/test/java/org/openecomp/core/externaltesting/impl/ExternalTestingManagerImplTest.java b/openecomp-be/lib/openecomp-sdc-externaltesting-lib/openecomp-sdc-externaltesting-impl/src/test/java/org/openecomp/core/externaltesting/impl/ExternalTestingManagerImplTest.java
index 04ddf6e2d4..c429709182 100644
--- a/openecomp-be/lib/openecomp-sdc-externaltesting-lib/openecomp-sdc-externaltesting-impl/src/test/java/org/openecomp/core/externaltesting/impl/ExternalTestingManagerImplTest.java
+++ b/openecomp-be/lib/openecomp-sdc-externaltesting-lib/openecomp-sdc-externaltesting-impl/src/test/java/org/openecomp/core/externaltesting/impl/ExternalTestingManagerImplTest.java
@@ -18,6 +18,16 @@ package org.openecomp.core.externaltesting.impl;
 
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Optional;
+import java.util.UUID;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.tuple.Pair;
@@ -25,28 +35,37 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.*;
+import org.mockito.ArgumentMatchers;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
 import org.mockito.junit.MockitoJUnitRunner;
-import org.openecomp.core.externaltesting.api.*;
+import org.openecomp.core.externaltesting.api.ClientConfiguration;
+import org.openecomp.core.externaltesting.api.ExternalTestingManager;
+import org.openecomp.core.externaltesting.api.RemoteTestingEndpointDefinition;
+import org.openecomp.core.externaltesting.api.TestTreeNode;
+import org.openecomp.core.externaltesting.api.VtpNameDescriptionPair;
+import org.openecomp.core.externaltesting.api.VtpTestCase;
+import org.openecomp.core.externaltesting.api.VtpTestExecutionRequest;
+import org.openecomp.core.externaltesting.api.VtpTestExecutionResponse;
 import org.openecomp.core.externaltesting.errors.ExternalTestingException;
 import org.openecomp.sdc.vendorsoftwareproduct.OrchestrationTemplateCandidateManager;
 import org.openecomp.sdc.vendorsoftwareproduct.VendorSoftwareProductManager;
 import org.openecomp.sdc.versioning.VersioningManager;
 import org.openecomp.sdc.versioning.dao.types.Version;
 import org.springframework.core.ParameterizedTypeReference;
-import org.springframework.http.*;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.web.client.HttpServerErrorException;
 import org.springframework.web.client.HttpStatusCodeException;
 import org.springframework.web.client.ResourceAccessException;
 import org.springframework.web.client.RestTemplate;
 
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.IOException;
-import java.nio.charset.Charset;
-import java.util.*;
-
 @RunWith(MockitoJUnitRunner.class)
 public class ExternalTestingManagerImplTest {