From 43f8b2f7421ebafe1db1eb1cf763998796eb06a8 Mon Sep 17 00:00:00 2001 From: alx652 Date: Thu, 13 Jun 2024 11:29:07 -0400 Subject: [PATCH 1/4] path traversal mitigation attempt snyk --- .../java/gsrs/config/FilePathParserUtils.java | 55 +++++++++++++ .../gsrs/controller/ExportController.java | 3 + .../gsrs/config/TestFileParserHelper.java | 77 +++++++++++++++++-- 3 files changed, 129 insertions(+), 6 deletions(-) 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..4ffbd586 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,56 @@ 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."); + } + } + +// Probably delete this +// public static void failIfDirectoryTraversal(String baseDirectoryPath , String relativePath) +// { +// File relativePathFile = Paths.get(relativePath).toFile(); +// if (relativePathFile.isAbsolute()) { +// throw new RuntimeException("Directory traversal attempt - absolute path not allowed"); +// } +// String combinedPathUsingCanonical; +// String combinedPathUsingAbsolute; +// +// try +// { +// combinedPathUsingCanonical =Paths.get(baseDirectoryPath, relativePath).toFile().getCanonicalPath(); +// combinedPathUsingAbsolute = Paths.get(baseDirectoryPath, relativePath).toFile().getAbsolutePath(); +// } +// catch (IOException e) +// { +// throw new RuntimeException("Directory traversal attempt?", e); +// } +// // Require the absolute path and canonicalized path match. +// // This is done to avoid directory traversal +// // attacks, e.g. "1/../2/" +// if (! combinedPathUsingCanonical.equals(combinedPathUsingAbsolute)) +// { +// throw new RuntimeException("Directory traversal attempt?"); +// } +// } + + + } 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..16fc8ccf 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,6 +185,8 @@ public ResponseEntity download(@PathVariable("id") @Pattern(regexp="^[a- File f = exportFile.get().getFile(); + // __aw__ come back + // FilePathParserUtils.failIfDirectoryTraversal(f.getPath()); Path path = Paths.get(f.getAbsolutePath()); 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..5de15632 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,14 +1,17 @@ package gsrs.config; -import static org.junit.jupiter.api.Assertions.assertEquals; - import java.io.File; import java.io.IOException; +import org.apache.commons.lang3.SystemUtils; import org.junit.jupiter.api.Test; import gsrs.config.FilePathParserUtils.FileParser; +import javax.swing.plaf.synth.SynthTextAreaUI; + +import static org.junit.jupiter.api.Assertions.*; + public class TestFileParserHelper { @@ -99,8 +102,6 @@ public void nullSuppliedGivenDefaultFilePathWithSetRelativeRootShouldBeEffective assertEquals(new File("test").toPath().normalize(), f.toPath().normalize()); } - - @Test public void nullSuppliedGivenDefaultRelativeWithSetRootShouldBeAbsolute() throws IOException { @@ -112,6 +113,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); + } + } From b93dc407c698774774bb24bdd4c57b90986a246b Mon Sep 17 00:00:00 2001 From: alx652 Date: Tue, 2 Jul 2024 14:21:06 -0400 Subject: [PATCH 2/4] clean up --- .../java/gsrs/config/FilePathParserUtils.java | 30 ------------------- 1 file changed, 30 deletions(-) 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 4ffbd586..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 @@ -103,34 +103,4 @@ public static void failOnBadPathResolution(String basePath, String passedPath) } } -// Probably delete this -// public static void failIfDirectoryTraversal(String baseDirectoryPath , String relativePath) -// { -// File relativePathFile = Paths.get(relativePath).toFile(); -// if (relativePathFile.isAbsolute()) { -// throw new RuntimeException("Directory traversal attempt - absolute path not allowed"); -// } -// String combinedPathUsingCanonical; -// String combinedPathUsingAbsolute; -// -// try -// { -// combinedPathUsingCanonical =Paths.get(baseDirectoryPath, relativePath).toFile().getCanonicalPath(); -// combinedPathUsingAbsolute = Paths.get(baseDirectoryPath, relativePath).toFile().getAbsolutePath(); -// } -// catch (IOException e) -// { -// throw new RuntimeException("Directory traversal attempt?", e); -// } -// // Require the absolute path and canonicalized path match. -// // This is done to avoid directory traversal -// // attacks, e.g. "1/../2/" -// if (! combinedPathUsingCanonical.equals(combinedPathUsingAbsolute)) -// { -// throw new RuntimeException("Directory traversal attempt?"); -// } -// } - - - } From 4934fd8440ad03bc1a3c7bf847f54b8dba24a7b4 Mon Sep 17 00:00:00 2001 From: alx652 Date: Tue, 2 Jul 2024 14:22:47 -0400 Subject: [PATCH 3/4] clean up --- .../src/main/java/gsrs/controller/ExportController.java | 4 ---- 1 file changed, 4 deletions(-) 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 16fc8ccf..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 @@ -185,10 +185,6 @@ public ResponseEntity download(@PathVariable("id") @Pattern(regexp="^[a- File f = exportFile.get().getFile(); - // __aw__ come back - // FilePathParserUtils.failIfDirectoryTraversal(f.getPath()); - - Path path = Paths.get(f.getAbsolutePath()); ByteArrayResource resource = new ByteArrayResource(Files.readAllBytes(path)); From 93a2cb22ddc20f45f1e857d8d8c6825d8cb671fd Mon Sep 17 00:00:00 2001 From: alx652 Date: Tue, 2 Jul 2024 14:37:16 -0400 Subject: [PATCH 4/4] clean up --- .../src/test/java/gsrs/config/TestFileParserHelper.java | 6 ------ 1 file changed, 6 deletions(-) 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 5de15632..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 @@ -2,14 +2,8 @@ import java.io.File; import java.io.IOException; - -import org.apache.commons.lang3.SystemUtils; import org.junit.jupiter.api.Test; - import gsrs.config.FilePathParserUtils.FileParser; - -import javax.swing.plaf.synth.SynthTextAreaUI; - import static org.junit.jupiter.api.Assertions.*;