diff --git a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/config/FilePathParserUtils.java b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/config/FilePathParserUtils.java index 89450101..fa764c74 100644 --- a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/config/FilePathParserUtils.java +++ b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/config/FilePathParserUtils.java @@ -1,7 +1,10 @@ package gsrs.config; import java.io.File; +import java.io.IOException; +import java.nio.file.InvalidPathException; import java.nio.file.Path; +import java.nio.file.Paths; import java.time.format.DateTimeFormatter; import gov.nih.ncats.common.util.TimeUtil; @@ -78,4 +81,26 @@ private static File getDefaultRootDir() { return null; } + public static void failOnBadPathResolution(String basePath, String passedPath) + throws InvalidPathException, IOException { + Path basePathObj; + Path resolvedPathObj; + try { + basePathObj = Paths.get(basePath); + resolvedPathObj = basePathObj.resolve(passedPath); +// System.out.println("getPath : " + resolvedPathObj.toFile().getPath()); +// System.out.println("getAbsPath : " + resolvedPathObj.toFile().getAbsolutePath()); +// System.out.println("getNormAbsPath: " + resolvedPathObj.normalize().toAbsolutePath()); + } catch (InvalidPathException ipe) { + // Don't want to provide input path as it might get flagged. + throw new InvalidPathException("Relating to passedPath", "Exception while resolving path"); + } + if (Paths.get(passedPath).isAbsolute()) { + throw new IOException("Absolute path not allowed."); + } + if (!resolvedPathObj.normalize().startsWith(basePathObj)) { + throw new IOException("Unexpected start of path."); + } + } + } diff --git a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/controller/ExportController.java b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/controller/ExportController.java index b577c29c..fd70dce1 100644 --- a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/controller/ExportController.java +++ b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/controller/ExportController.java @@ -1,5 +1,6 @@ package gsrs.controller; +import gsrs.config.FilePathParserUtils; import gsrs.service.ExportService; import ix.ginas.exporters.ExportDir; import ix.ginas.exporters.ExportMetaData; @@ -184,8 +185,6 @@ public ResponseEntity download(@PathVariable("id") @Pattern(regexp="^[a- File f = exportFile.get().getFile(); - - Path path = Paths.get(f.getAbsolutePath()); ByteArrayResource resource = new ByteArrayResource(Files.readAllBytes(path)); diff --git a/gsrs-spring-boot-autoconfigure/src/test/java/gsrs/config/TestFileParserHelper.java b/gsrs-spring-boot-autoconfigure/src/test/java/gsrs/config/TestFileParserHelper.java index 47402423..ba2a0949 100644 --- a/gsrs-spring-boot-autoconfigure/src/test/java/gsrs/config/TestFileParserHelper.java +++ b/gsrs-spring-boot-autoconfigure/src/test/java/gsrs/config/TestFileParserHelper.java @@ -1,13 +1,10 @@ package gsrs.config; -import static org.junit.jupiter.api.Assertions.assertEquals; - import java.io.File; import java.io.IOException; - import org.junit.jupiter.api.Test; - import gsrs.config.FilePathParserUtils.FileParser; +import static org.junit.jupiter.api.Assertions.*; public class TestFileParserHelper { @@ -99,8 +96,6 @@ public void nullSuppliedGivenDefaultFilePathWithSetRelativeRootShouldBeEffective assertEquals(new File("test").toPath().normalize(), f.toPath().normalize()); } - - @Test public void nullSuppliedGivenDefaultRelativeWithSetRootShouldBeAbsolute() throws IOException { @@ -112,6 +107,70 @@ public void nullSuppliedGivenDefaultRelativeWithSetRootShouldBeAbsolute() throws assertEquals(new File("/tmp/test"), f); } - - + + @Test + public void testFailOnBadPathResolutionNormal() throws IOException { + boolean hasException = false; + try { + // Should succeed since normalized path ok + FilePathParserUtils.failOnBadPathResolution("/tmp", "my-file.txt"); + } catch(Exception e) { + System.out.println(e.getMessage()); + hasException = true; + } + assertFalse(hasException); + } + + @Test + public void testFailOnBadPathResolutionSingleDot1() throws IOException { + boolean hasException = false; + try { + // Should succeed since path resolves to /tmp/my-file.txt + FilePathParserUtils.failOnBadPathResolution("/tmp", "./my-file.txt"); + } catch(Exception e) { + System.out.println(e.getMessage()); + hasException = true; + } + assertFalse(hasException); + } + + @Test + public void testFailOnBadPathResolutionDotDot1() throws IOException { + boolean hasException = false; + try { + // Should succeed since path resolves to /tmp/my-file.txt + FilePathParserUtils.failOnBadPathResolution("/tmp", "../tmp/my-file.txt"); + } catch(Exception e) { + System.out.println(e.getMessage()); + hasException = true; + } + assertFalse(hasException); + } + + @Test + public void testFailOnBadPathResolutionDotDot2() throws IOException { + boolean hasException = false; + try { + // Should fail since path resolves to /etc/my-file.txt + FilePathParserUtils.failOnBadPathResolution("/tmp", "../etc/my-file.txt"); + } catch(Exception e) { + System.out.println(e.getMessage()); + hasException = true; + } + assertTrue(hasException); + } + + @Test + public void testFailOnBadPathResolutionAbsolutePathException() throws IOException { + boolean hasException = false; + try { + // Should fail due to absolute path + FilePathParserUtils.failOnBadPathResolution("/tmp", "/my/abs/path"); + } catch(Exception e) { + System.out.println(e.getMessage()); + hasException = true; + } + assertTrue(hasException); + } + }