aboutsummaryrefslogtreecommitdiffstats
path: root/openecomp-be/lib/openecomp-common-lib
diff options
context:
space:
mode:
authorandre.schmid <andre.schmid@est.tech>2019-09-27 13:27:11 +0100
committerOfir Sonsino <ofir.sonsino@intl.att.com>2019-10-30 09:47:54 +0000
commitbf5eeb23a769a2e2b75f432b74f10fdbcfd2f161 (patch)
treefa27998ee6efef6f7651315cbf71271130fca025 /openecomp-be/lib/openecomp-common-lib
parent19773b769c6762a12876064c70a34cc31d2b12da (diff)
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>
Diffstat (limited to 'openecomp-be/lib/openecomp-common-lib')
-rw-r--r--openecomp-be/lib/openecomp-common-lib/src/main/java/org/openecomp/sdc/common/utils/CommonUtil.java118
-rw-r--r--openecomp-be/lib/openecomp-common-lib/src/test/java/org/openecomp/sdc/common/utils/CommonUtilTest.java18
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));
}
+
}