diff options
author | Remigiusz Janeczek <remigiusz.janeczek@nokia.com> | 2021-03-19 09:21:42 +0100 |
---|---|---|
committer | Remigiusz Janeczek <remigiusz.janeczek@nokia.com> | 2021-03-22 13:37:43 +0100 |
commit | 25763727265dd20b0301db0164c3e5549700cab4 (patch) | |
tree | 35b80e227db0306bca2f2ff9c6af03aa255ee9f5 | |
parent | 92c83fe593132a522cb26a872d94612373e46315 (diff) |
Fix path from user input vulnerability
Issue-ID: SDC-3185
Signed-off-by: Remigiusz Janeczek <remigiusz.janeczek@nokia.com>
Change-Id: I9c121d2ba24487e45d08d2937cbc7bddc07e163e
-rw-r--r-- | .gitignore | 2 | ||||
-rw-r--r-- | src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java | 26 | ||||
-rw-r--r-- | src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java | 21 |
3 files changed, 7 insertions, 42 deletions
@@ -4,7 +4,7 @@ target/ !**/src/test/**/target/ /scripts/helm_tmp/* /scripts/helm_versions/* - +/src/test/resources/tmp ### IntelliJ IDEA ### .idea diff --git a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java index 4bc8ea6..ab1d186 100644 --- a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java +++ b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java @@ -20,12 +20,11 @@ package org.onap.sdc.helmvalidator.helm.validation; -import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; -import java.time.Instant; import org.onap.sdc.helmvalidator.helm.validation.exception.SaveFileException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,12 +46,11 @@ public class FileManager { String saveFile(MultipartFile file) { LOGGER.debug("Base PATH: {}", basePath); - try { - String filePath = basePath + File.separator + generateFileName(file.getOriginalFilename()); - LOGGER.info("Attempt to save file : {}", filePath); - Files.copy(file.getInputStream(), Paths.get(filePath), StandardCopyOption.REPLACE_EXISTING); - return filePath; + final Path tmpFilePath = Files.createTempFile(Paths.get(basePath), "chart-", ".tgz"); + LOGGER.info("Attempt to save file : {}", tmpFilePath); + Files.copy(file.getInputStream(), tmpFilePath, StandardCopyOption.REPLACE_EXISTING); + return tmpFilePath.toString(); } catch (IOException e) { throw new SaveFileException("Cannot save file: " + file.getOriginalFilename(), e); } @@ -66,18 +64,4 @@ public class FileManager { LOGGER.warn("Cannot delete file: {}, Exception: {}", path, e.getStackTrace()); } } - - private String generateFileName(String fileName) { - if (isValidFileName(fileName)) { - return Instant.now().toEpochMilli() + "_" + fileName.replaceAll("\\s+", ""); - } - throw new SaveFileException("Not allowed file name"); - } - - private boolean isValidFileName(String fileName) { - if (fileName == null){ - return false; - } - return !fileName.contains("/"); - } } diff --git a/src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java b/src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java index 89eab80..8057303 100644 --- a/src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java +++ b/src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java @@ -47,7 +47,6 @@ class FileManagerTest { private static final String TEST_RESOURCES_TMP = "src/test/resources/tmp"; private static final File TEST_RESOURCES_DIR = new File(TEST_RESOURCES_TMP); private static final ByteArrayInputStream TEST_INPUT_STREAM = new ByteArrayInputStream("test".getBytes()); - private static final String SAMPLE_FILE_NAME = "sample_file"; private FileManager fileManager; @@ -71,7 +70,7 @@ class FileManagerTest { @BeforeEach void setUp() { - fileManager = new FileManager(TEST_RESOURCES_TMP); + fileManager = new FileManager(TEST_RESOURCES_DIR.getAbsolutePath()); } @Test @@ -95,25 +94,7 @@ class FileManagerTest { assertThat(Files.exists(Paths.get(filePath))).isFalse(); } - @Test - void shouldThrowExceptionWhenFileContainsSlash() { - when(multipartFile.getOriginalFilename()).thenReturn(SAMPLE_FILE_NAME + "/"); - Exception exception = assertThrows(SaveFileException.class, - () -> fileManager.saveFile(multipartFile)); - assertThat(exception).hasMessageContaining("Not allowed file name"); - } - - @Test - void shouldThrowExceptionWhenFileNameIsNull() { - when(multipartFile.getOriginalFilename()).thenReturn(null); - - Exception exception = assertThrows(SaveFileException.class, - () -> fileManager.saveFile(multipartFile)); - assertThat(exception).hasMessageContaining("Not allowed file name"); - } - private void mockMultipartFile() throws IOException { - when(multipartFile.getOriginalFilename()).thenReturn(SAMPLE_FILE_NAME); when(multipartFile.getInputStream()).thenReturn(TEST_INPUT_STREAM); } } |