aboutsummaryrefslogtreecommitdiffstats
path: root/openecomp-be/backend
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/backend
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/backend')
-rw-r--r--openecomp-be/backend/openecomp-sdc-validation-manager/src/main/java/org/openecomp/sdc/validation/impl/UploadValidationManagerImpl.java78
-rw-r--r--openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/main/java/org/openecomp/sdc/vendorsoftwareproduct/impl/orchestration/OrchestrationTemplateCSARHandler.java3
-rw-r--r--openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/test/java/org/openecomp/sdc/vendorsoftwareproduct/upload/csar/UploadCSARFileTest.java4
3 files changed, 25 insertions, 60 deletions
diff --git a/openecomp-be/backend/openecomp-sdc-validation-manager/src/main/java/org/openecomp/sdc/validation/impl/UploadValidationManagerImpl.java b/openecomp-be/backend/openecomp-sdc-validation-manager/src/main/java/org/openecomp/sdc/validation/impl/UploadValidationManagerImpl.java
index 42c77b96d8..90f8ad369b 100644
--- a/openecomp-be/backend/openecomp-sdc-validation-manager/src/main/java/org/openecomp/sdc/validation/impl/UploadValidationManagerImpl.java
+++ b/openecomp-be/backend/openecomp-sdc-validation-manager/src/main/java/org/openecomp/sdc/validation/impl/UploadValidationManagerImpl.java
@@ -20,7 +20,11 @@
package org.openecomp.sdc.validation.impl;
-import org.apache.commons.collections4.CollectionUtils;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
import org.apache.commons.collections4.MapUtils;
import org.openecomp.core.utilities.file.FileContentHandler;
import org.openecomp.core.utilities.file.FileUtils;
@@ -30,6 +34,8 @@ import org.openecomp.sdc.common.errors.ErrorCategory;
import org.openecomp.sdc.common.errors.ErrorCode;
import org.openecomp.sdc.common.errors.Messages;
import org.openecomp.sdc.common.utils.SdcCommon;
+import org.openecomp.sdc.common.zip.ZipUtils;
+import org.openecomp.sdc.common.zip.exception.ZipException;
import org.openecomp.sdc.datatypes.error.ErrorMessage;
import org.openecomp.sdc.heat.datatypes.structure.ValidationStructureList;
import org.openecomp.sdc.heat.services.tree.HeatTreeManager;
@@ -38,79 +44,35 @@ import org.openecomp.sdc.validation.UploadValidationManager;
import org.openecomp.sdc.validation.types.ValidationFileResponse;
import org.openecomp.sdc.validation.util.ValidationManagerUtil;
-import java.io.ByteArrayInputStream;
-import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipInputStream;
-
/**
* Created by TALIO on 4/20/2016.
*/
public class UploadValidationManagerImpl implements UploadValidationManager {
- private static FileContentHandler getFileContentMapFromZip(byte[] uploadFileData)
- throws IOException, CoreException {
-
- ZipEntry zipEntry;
- List<String> folderList = new ArrayList<>();
- FileContentHandler mapFileContent = new FileContentHandler();
- try (ZipInputStream inputZipStream = new ZipInputStream(new ByteArrayInputStream(uploadFileData))) {
-
- byte[] fileByteContent;
- String currentEntryName;
-
- while ((zipEntry = inputZipStream.getNextEntry()) != null) {
- currentEntryName = zipEntry.getName();
- // else, get the file content (as byte array) and save it in a map.
- fileByteContent = FileUtils.toByteArray(inputZipStream);
-
- int index = lastIndexFileSeparatorIndex(currentEntryName);
- String currSubstringWithoutSeparator =
- currentEntryName.substring(index + 1, currentEntryName.length());
- if (index != -1) {
- if (currSubstringWithoutSeparator.length() > 0) {
- mapFileContent.addFile(currentEntryName.substring(index + 1, currentEntryName.length()),
- fileByteContent);
- } else {
- folderList.add(currentEntryName);
- }
- } else {
- mapFileContent.addFile(currentEntryName, fileByteContent);
- }
- }
- } catch (RuntimeException exception) {
- throw new IOException(exception);
+
+ private static FileContentHandler getFileContentMapFromZip(byte[] uploadFileData) throws IOException {
+ final Map<String, byte[]> zipFileAndByteMap;
+ try {
+ zipFileAndByteMap = ZipUtils.readZip(uploadFileData, true);
+ } catch (final ZipException e) {
+ throw new IOException(e);
}
- if (CollectionUtils.isNotEmpty(folderList)) {
+ final boolean zipHasFolders = zipFileAndByteMap.values().stream().anyMatch(Objects::isNull);
+ if (zipHasFolders) {
throw new CoreException((new ErrorCode.ErrorCodeBuilder())
.withMessage(Messages.ZIP_SHOULD_NOT_CONTAIN_FOLDERS.getErrorMessage())
.withId(Messages.ZIP_SHOULD_NOT_CONTAIN_FOLDERS.getErrorMessage())
.withCategory(ErrorCategory.APPLICATION).build());
-
}
+ final FileContentHandler mapFileContent = new FileContentHandler();
+ zipFileAndByteMap.entrySet().stream()
+ .filter(entry -> entry.getValue() != null)
+ .forEach(zipEntry -> mapFileContent.addFile(zipEntry.getKey(), zipEntry.getValue()));
return mapFileContent;
}
- 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;
- }
-
@Override
public ValidationFileResponse validateFile(String type, InputStream fileToValidate)
throws IOException {
diff --git a/openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/main/java/org/openecomp/sdc/vendorsoftwareproduct/impl/orchestration/OrchestrationTemplateCSARHandler.java b/openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/main/java/org/openecomp/sdc/vendorsoftwareproduct/impl/orchestration/OrchestrationTemplateCSARHandler.java
index 61d1799aa0..8f0029aa83 100644
--- a/openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/main/java/org/openecomp/sdc/vendorsoftwareproduct/impl/orchestration/OrchestrationTemplateCSARHandler.java
+++ b/openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/main/java/org/openecomp/sdc/vendorsoftwareproduct/impl/orchestration/OrchestrationTemplateCSARHandler.java
@@ -32,6 +32,7 @@ import org.openecomp.sdc.common.errors.CoreException;
import org.openecomp.sdc.common.errors.Messages;
import org.openecomp.sdc.common.utils.CommonUtil;
import org.openecomp.sdc.common.utils.SdcCommon;
+import org.openecomp.sdc.common.zip.exception.ZipException;
import org.openecomp.sdc.datatypes.error.ErrorLevel;
import org.openecomp.sdc.datatypes.error.ErrorMessage;
import org.openecomp.sdc.vendorsoftwareproduct.dao.type.OrchestrationTemplateCandidateData;
@@ -58,7 +59,7 @@ public class OrchestrationTemplateCSARHandler extends BaseOrchestrationTemplateH
folderList = fileContentMapFromOrchestrationCandidateZip.getRight();
Validator validator = ValidatorFactory.getValidator(contentMap);
uploadFileResponse.addStructureErrors(validator.validateContent(contentMap, folderList));
- } catch (IOException exception) {
+ } catch (final ZipException | IOException exception) {
logger.error(exception.getMessage(), exception);
uploadFileResponse.addStructureError(
SdcCommon.UPLOAD_FILE,
diff --git a/openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/test/java/org/openecomp/sdc/vendorsoftwareproduct/upload/csar/UploadCSARFileTest.java b/openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/test/java/org/openecomp/sdc/vendorsoftwareproduct/upload/csar/UploadCSARFileTest.java
index 122809896e..7a183c0dbb 100644
--- a/openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/test/java/org/openecomp/sdc/vendorsoftwareproduct/upload/csar/UploadCSARFileTest.java
+++ b/openecomp-be/backend/openecomp-sdc-vendor-software-product-manager/src/test/java/org/openecomp/sdc/vendorsoftwareproduct/upload/csar/UploadCSARFileTest.java
@@ -18,7 +18,9 @@ package org.openecomp.sdc.vendorsoftwareproduct.upload.csar;
import static junit.framework.TestCase.assertTrue;
+import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
@@ -161,7 +163,7 @@ public class UploadCSARFileTest {
onboardPackageInfo = new OnboardPackageInfo(csarFileName, OnboardingTypesEnum.CSAR.toString(),
convertFileInputStream(inputStream));
uploadFileResponse = candidateManager.upload(vspDetails, onboardPackageInfo);
- assertEquals(expectedErrorsNumber, uploadFileResponse.getErrors().size());
+ assertThat(String.format("Expecting %s error(s) in file '%s'", expectedErrorsNumber, csarFileName), uploadFileResponse.getErrors().size(), is(expectedErrorsNumber));
}
return uploadFileResponse;
}