aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRemigiusz Janeczek <remigiusz.janeczek@nokia.com>2021-03-19 09:21:42 +0100
committerRemigiusz Janeczek <remigiusz.janeczek@nokia.com>2021-03-22 13:37:43 +0100
commit25763727265dd20b0301db0164c3e5549700cab4 (patch)
tree35b80e227db0306bca2f2ff9c6af03aa255ee9f5
parent92c83fe593132a522cb26a872d94612373e46315 (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--.gitignore2
-rw-r--r--src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java26
-rw-r--r--src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java21
3 files changed, 7 insertions, 42 deletions
diff --git a/.gitignore b/.gitignore
index 23cc2f4..15e77b8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -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);
}
}