From bc9ca62a86e3f8c7d58450054d68ee9d11e63cdb Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sat, 22 Aug 2026 23:45:35 +0100 Subject: [PATCH] compare path elements when checking that a file is below the served directory Motivation: `checkIsSafeDescendant` compared the canonical location of the requested file with the canonical path of the served directory as a plain string prefix. A path such as `/var/www-private/secret` has `/var/www` as a string prefix without being contained in it, so a symbolic link inside the served directory that resolves to such a sibling directory passed the check and the file was served. The segment filter in `safeJoinPaths` does not catch this, because no path segment is suspicious; only canonicalization moves the location out of the directory. Modification: Compare the two canonical paths element by element via `java.nio.file.Path` instead of as strings. That keeps the base directory itself accepted, which the directory listing of `getFromBrowseableDirectory` relies on. Result: Only files that are really below the served directory are served, and a symbolic link to a sibling directory is rejected with the existing warning regardless of how the sibling is named. Tests: - sbt "http-tests/testOnly org.apache.pekko.http.scaladsl.server.directives.FileAndResourceDirectivesSymlinkSpec" - pass, 1 new test that serves a symlink to a sibling directory named after the served one; it fails without the change - sbt http-tests/test - pass - sbt http/mimaReportBinaryIssues - pass - sbt http/scalafmt http-tests/Test/scalafmt - clean References: None - tightens the containment check for file and resource directives --- ...FileAndResourceDirectivesSymlinkSpec.scala | 24 +++++++++++++++++++ .../FileAndResourceDirectives.scala | 6 ++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSymlinkSpec.scala b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSymlinkSpec.scala index 42f9324bf2..f72a3abf19 100644 --- a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSymlinkSpec.scala +++ b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSymlinkSpec.scala @@ -15,6 +15,7 @@ package org.apache.pekko.http.scaladsl.server package directives import java.io.File +import java.nio.charset.StandardCharsets import java.nio.file.{ Files, Paths } import scala.concurrent.duration._ @@ -41,9 +42,21 @@ class FileAndResourceDirectivesSymlinkSpec extends RoutingSpec Paths.get(dirWithLink.getAbsolutePath, "linked-dir"), new File(testRoot, "subDirectory").toPath.toAbsolutePath) + // a sibling of the served directory whose name has the name of the served directory as a prefix + val siblingDir = new File(tempDir.toFile, "dirWithLink-private") + siblingDir.mkdir() + val siblingFile = new File(siblingDir, "secret.txt") + Files.write(siblingFile.toPath, "secret".getBytes(StandardCharsets.UTF_8)) + val siblingSymlink = Files.createSymbolicLink( + Paths.get(dirWithLink.getAbsolutePath, "linked-sibling"), + siblingDir.toPath.toAbsolutePath) + override def afterAll(): Unit = { super.afterAll() Files.deleteIfExists(symlink) + Files.deleteIfExists(siblingSymlink) + Files.deleteIfExists(siblingFile.toPath) + Files.deleteIfExists(siblingDir.toPath) Files.deleteIfExists(dirWithLink.toPath) Files.deleteIfExists(tempDir) } @@ -69,5 +82,16 @@ class FileAndResourceDirectivesSymlinkSpec extends RoutingSpec } } } + + "not follow symbolic links into a sibling directory whose name starts with the served directory" in { + Files.isSymbolicLink(siblingSymlink) shouldBe true + // the canonical location of the file is `/dirWithLink-private/secret.txt`, which has the canonical + // path of the served directory, `/dirWithLink`, as a string prefix + EventFilter.warning(pattern = ".* points to a location that is not part of .*", occurrences = 1).intercept { + Get("linked-sibling/secret.txt") ~> _getFromDirectory() ~> check { + handled shouldBe false + } + } + } } } diff --git a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala index 19a6c99551..53c7fcc300 100644 --- a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala +++ b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala @@ -16,6 +16,7 @@ package directives import java.io.File import java.net.{ URI, URL } +import java.nio.file.Paths import scala.annotation.tailrec import scala.jdk.CollectionConverters._ @@ -270,7 +271,10 @@ object FileAndResourceDirectives extends FileAndResourceDirectives { val finalFile = new File(finalPath) val canonicalFinalPath = finalFile.getCanonicalPath - if (!canonicalFinalPath.startsWith(baseFile.getCanonicalPath)) { + // compared element by element instead of as plain strings: `/var/www-private/secret` has the canonical path of + // `/var/www` as a string prefix without being contained in that directory, which canonicalization can produce + // for a symbolic link that points at a sibling directory + if (!Paths.get(canonicalFinalPath).startsWith(Paths.get(baseFile.getCanonicalPath))) { log.warning("[{}] points to a location that is not part of [{}]. This might be a directory traversal attempt.", finalFile, baseFile) ""