CVE-2021-21234: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: spring-boot-actuator-logview in a library that adds a simple logfile viewer as spring boot actuator endpoint. It is maven package "eu.hinsch:spring-boot-actuator-logview". In spring-boot-actuator-logview before version 0.2.13 there is a directory traversal vulnerability. The nature of this library is to expose a log file directory via admin (spring boot actuator) HTTP endpoints. Both the filename to view and a base folder (relative to the logging folder root) can be specified via request parameters. While the filename parameter was checked to prevent directory traversal exploits (so that `filename=../somefile` would not work), the base folder parameter was not sufficiently checked, so that `filename=somefile&base=../` could access a file outside the logging base directory). The vulnerability has been patched in release 0.2.13. Any users of 0.2.12 should be able to update without any issues as there are no other changes in that release. There is no workaround to fix the vulnerability other than updating or removing the dependency. However, removing read access of the user the application is run with to any directory not required for running the application can limit the impact. Additionally, access to the logview endpoint can be limited by deploying the application behind a reverse proxy.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 7.7
Published: 2021-01-05T18:15Z
Repository: https://github.com/lukashinsch/spring-boot-actuator-logview
Language: Java
Files changed: .gitignore, .travis.yml, lib/src/main/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpoint.java, lib/src/test/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpointTest.java
Commit date: 2021-01-01 22:36:07 +0100
Commit message: backport directory traversal vulnerability fix

Diff: diff --git a/.gitignore b/.gitignore
index ff415b3..2279dbe 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,5 @@
 .gradle
 .idea
 *.iml
 build/
+out/
diff --git a/.travis.yml b/.travis.yml
index c6db5ba..2de67b2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,6 +1,6 @@
 language: java
 jdk:
-  - oraclejdk8
+  - openjdk8
 
 after_success:
   - ./gradlew lib:jacocoTestReport lib:coveralls
\ No newline at end of file
diff --git a/lib/src/main/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpoint.java b/lib/src/main/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpoint.java
index 1bef7ce..493f07d 100644
--- a/lib/src/main/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpoint.java
+++ b/lib/src/main/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpoint.java
@@ -58,13 +58,13 @@ public class LogViewEndpoint implements MvcEndpoint{
     @ResponseBody
     public String list(Model model, // TODO model should no longer be injected
                        @RequestParam(required = false, defaultValue = "FILENAME") SortBy sortBy,
                        @RequestParam(required = false, defaultValue = "false") boolean desc,
                        @RequestParam(required = false) String base) throws IOException, TemplateException {
-        securityCheck(base);
-
         Path currentFolder = loggingPath(base);
+        securityCheck(currentFolder, null);
+
 
         List<FileEntry> files = getFileProvider(currentFolder).getFileEntries(currentFolder);
         List<FileEntry> sortedFiles = sortFiles(files, sortBy, desc);
 
         model.addAttribute("sortBy", sortBy);
@@ -125,14 +125,14 @@ public class LogViewEndpoint implements MvcEndpoint{
     @RequestMapping("/view")
     public void view(@RequestParam String filename,
                      @RequestParam(required = false) String base,
                      @RequestParam(required = false) Integer tailLines,
                      HttpServletResponse response) throws IOException {
-        securityCheck(filename);
-        response.setContentType(MediaType.TEXT_PLAIN_VALUE);
 
         Path path = loggingPath(base);
+        securityCheck(path, filename);
+        response.setContentType(MediaType.TEXT_PLAIN_VALUE);
         FileProvider fileProvider = getFileProvider(path);
         if (tailLines != null) {
             fileProvider.tailContent(path, filename, response.getOutputStream(), tailLines);
         }
         else {
@@ -169,12 +169,19 @@ public class LogViewEndpoint implements MvcEndpoint{
         } catch (IOException e) {
             throw new RuntimeException("error reading file", e);
         }
     }
 
-    private void securityCheck(String filename) {
-        Assert.doesNotContain(filename, "..");
+    private void securityCheck(Path base, String filename) {
+        try {
+            String canonicalLoggingPath = (filename != null ? new File(base.toFile().toString(), filename) : new File(base.toFile().toString())).getCanonicalPath();
+            String baseCanonicalPath = new File(loggingPath).getCanonicalPath();
+            String errorMessage = "File " + base.toString() + "/" + filename + " may not be located outside base path " + loggingPath;
+            Assert.isTrue(canonicalLoggingPath.startsWith(baseCanonicalPath), errorMessage);
+        } catch (IOException e) {
+            throw new IllegalStateException(e);
+        }
     }
 
     @Override
     public String getPath() {
         return "/log";
diff --git a/lib/src/test/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpointTest.java b/lib/src/test/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpointTest.java
index 0c73325..fbbb1c9 100644
--- a/lib/src/test/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpointTest.java
+++ b/lib/src/test/java/eu/hinsch/spring/boot/actuator/logview/LogViewEndpointTest.java
@@ -328,16 +328,26 @@ public class LogViewEndpointTest {
 
     @Test
     public void shouldNotAllowToListFileOutsideRoot() throws Exception {
         // given
         expectedException.expect(IllegalArgumentException.class);
-        expectedException.expectMessage(containsString("this String argument must not contain the substring [..]"));
+        expectedException.expectMessage(containsString("may not be located outside base path"));
 
         // when
         logViewEndpoint.view("../somefile", null, null, null);
     }
 
+    @Test
+    public void shouldNotAllowToListWithBaseOutsideRoot() throws Exception {
+        // given
+        expectedException.expect(IllegalArgumentException.class);
+        expectedException.expectMessage(containsString("may not be located outside base path"));
+
+        // when
+        logViewEndpoint.view("somefile", "../otherdir", null, null);
+    }
+
     @Test
     public void shouldViewFile() throws Exception {
         // given
         createFile("file.log", "abc", now);
         ByteArrayServletOutputStream outputStream = mockResponseOutputStream();

Vulnerable code:   - oraclejdk8
        securityCheck(base);

        securityCheck(filename);
        response.setContentType(MediaType.TEXT_PLAIN_VALUE);
    private void securityCheck(String filename) {
        Assert.doesNotContain(filename, "..");
        expectedException.expectMessage(containsString("this String argument must not contain the substring [..]"));
Fixed code: out/
  - openjdk8
        securityCheck(currentFolder, null);

        securityCheck(path, filename);
        response.setContentType(MediaType.TEXT_PLAIN_VALUE);
    private void securityCheck(Path base, String filename) {
        try {
            String canonicalLoggingPath = (filename != null ? new File(base.toFile().toString(), filename) : new File(base.toFile().toString())).getCanonicalPath();
            String baseCanonicalPath = new File(loggingPath).getCanonicalPath();
            String errorMessage = "File " + base.toString() + "/" + filename + " may not be located outside base path " + loggingPath;
            Assert.isTrue(canonicalLoggingPath.startsWith(baseCanonicalPath), errorMessage);
        } catch (IOException e) {
            throw new IllegalStateException(e);
        }
        expectedException.expectMessage(containsString("may not be located outside base path"));
    @Test
    public void shouldNotAllowToListWithBaseOutsideRoot() throws Exception {
        // given
        expectedException.expect(IllegalArgumentException.class);
        expectedException.expectMessage(containsString("may not be located outside base path"));

        // when
        logViewEndpoint.view("somefile", "../otherdir", null, null);
    }