CVE-2020-15230: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Vapor is a web framework for Swift. In Vapor before version 4.29.4, Attackers can access data at arbitrary filesystem paths on the same host as an application. Only applications using FileMiddleware are affected. This is fixed in version 4.29.4.
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: 6.5
Published: 2020-10-02T19:15Z
Repository: https://github.com/vapor/vapor
Language: Swift
Files changed: Sources/Vapor/Middleware/FileMiddleware.swift, Tests/VaporTests/FileTests.swift, Tests/VaporTests/Utilities/foo.txt
Commit date: 2020-09-30 19:28:20 -0400
Commit message: fix relative percent decoding in file middleware (#2500)


Diff: diff --git a/Sources/Vapor/Middleware/FileMiddleware.swift b/Sources/Vapor/Middleware/FileMiddleware.swift
index 0b3a20b8..3a9d0bfe 100644
--- a/Sources/Vapor/Middleware/FileMiddleware.swift
+++ b/Sources/Vapor/Middleware/FileMiddleware.swift
@@ -11,12 +11,14 @@ public final class FileMiddleware: Middleware {
         self.publicDirectory = publicDirectory.hasSuffix("/") ? publicDirectory : publicDirectory + "/"
     }
 
     /// See `Middleware`.
     public func respond(to request: Request, chainingTo next: Responder) -> EventLoopFuture<Response> {
-        // make a copy of the path
-        var path = request.url.path
+        // make a copy of the percent-decoded path
+        guard var path = request.url.path.removingPercentEncoding else {
+            return request.eventLoop.makeFailedFuture(Abort(.badRequest))
+        }
 
         // path must be relative.
         while path.hasPrefix("/") {
             path = String(path.dropFirst())
         }
@@ -25,11 +27,11 @@ public final class FileMiddleware: Middleware {
         guard !path.contains("../") else {
             return request.eventLoop.makeFailedFuture(Abort(.forbidden))
         }
 
         // create absolute file path
-        let filePath = self.publicDirectory + (path.removingPercentEncoding ?? path)
+        let filePath = self.publicDirectory + path
 
         // check if file exists and is not a directory
         var isDir: ObjCBool = false
         guard FileManager.default.fileExists(atPath: filePath, isDirectory: &isDir), !isDir.boolValue else {
             return next.respond(to: request)
diff --git a/Tests/VaporTests/FileTests.swift b/Tests/VaporTests/FileTests.swift
index be9a1a9b..a7cd43b8 100644
--- a/Tests/VaporTests/FileTests.swift
+++ b/Tests/VaporTests/FileTests.swift
@@ -59,6 +59,21 @@ final class FileTests: XCTestCase {
         try app.test(.GET, "/Utilities/foo%20bar.html") { res in
             XCTAssertEqual(res.status, .ok)
             XCTAssertEqual(res.body.string, "<h1>Hello</h1>\n")
         }
     }
+
+    func testPercentDecodedRelativePath() throws {
+        let app = Application(.testing)
+        defer { app.shutdown() }
+
+        let path = #file.split(separator: "/").dropLast().joined(separator: "/")
+        app.middleware.use(FileMiddleware(publicDirectory: "/" + path))
+
+        try app.test(.GET, "%2e%2e/VaporTests/Utilities/foo.txt") { res in
+            XCTAssertEqual(res.status, .forbidden)
+        }.test(.GET, "Utilities/foo.txt") { res in
+            XCTAssertEqual(res.status, .ok)
+            XCTAssertEqual(res.body.string, "bar\n")
+        }
+    }
 }
diff --git a/Tests/VaporTests/Utilities/foo.txt b/Tests/VaporTests/Utilities/foo.txt
new file mode 100644
index 00000000..5716ca59
--- /dev/null
+++ b/Tests/VaporTests/Utilities/foo.txt
@@ -0,0 +1 @@
+bar

Vulnerable code:         // make a copy of the path
        var path = request.url.path
        let filePath = self.publicDirectory + (path.removingPercentEncoding ?? path)
Fixed code:         // make a copy of the percent-decoded path
        guard var path = request.url.path.removingPercentEncoding else {
            return request.eventLoop.makeFailedFuture(Abort(.badRequest))
        }
        let filePath = self.publicDirectory + path

    func testPercentDecodedRelativePath() throws {
        let app = Application(.testing)
        defer { app.shutdown() }

        let path = #file.split(separator: "/").dropLast().joined(separator: "/")
        app.middleware.use(FileMiddleware(publicDirectory: "/" + path))

        try app.test(.GET, "%2e%2e/VaporTests/Utilities/foo.txt") { res in
            XCTAssertEqual(res.status, .forbidden)
        }.test(.GET, "Utilities/foo.txt") { res in
            XCTAssertEqual(res.status, .ok)
            XCTAssertEqual(res.body.string, "bar\n")
        }
    }
bar