Skip to content

Compare path elements when checking that a served file is below the base directory #1218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pjfanning wants to merge 1 commit into
base: main
Choose a base branch
from
+29 −1
Open

Compare path elements when checking that a served file is below the base directory #1218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions ...g/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSymlinkSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand All @@ -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)
}
Expand All @@ -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 `<tmp>/dirWithLink-private/secret.txt`, which has the canonical
// path of the served directory, `<tmp>/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
}
}
}
}
}
6 changes: 5 additions & 1 deletion ...in/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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)
""
Expand Down