Fix path from user input vulnerability

Issue-ID: SDC-3185
Signed-off-by: Remigiusz Janeczek <remigiusz.janeczek@nokia.com>
Change-Id: I9c121d2ba24487e45d08d2937cbc7bddc07e163e

2 files changed, 6 insertions, 41 deletions

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);
     }
 }