WASI: Implement path_open with proper symlink resolution

kateinoigakukun · kateinoigakukun · commit 6c2ccaf5ac5e · 2024-10-27T01:43:20.000+09:00
diff --git a/Sources/WASI/Platform/PlatformTypes.swift b/Sources/WASI/Platform/PlatformTypes.swift
@@ -156,15 +156,23 @@ extension WASIAbi.Errno {
         do {
             return try body()
         } catch let errno as Errno {
-            guard let error = WASIAbi.Errno(platformErrno: errno) else {
-                throw WASIError(description: "Unknown underlying OS error: \(errno)")
-            }
-            throw error
+            throw try WASIAbi.Errno(platformErrno: errno)
+        }
+    }
+
+    init(platformErrno: CInt) throws {
+        try self.init(platformErrno: SystemPackage.Errno(rawValue: platformErrno))
+    }
+
+    init(platformErrno: Errno) throws {
+        guard let error = WASIAbi.Errno(_platformErrno: platformErrno) else {
+            throw WASIError(description: "Unknown underlying OS error: \(platformErrno)")
         }
+        self = error
     }
 
-    init?(platformErrno: SystemPackage.Errno) {
-        switch platformErrno {
+    private init?(_platformErrno: SystemPackage.Errno) {
+        switch _platformErrno {
         case .permissionDenied: self = .EPERM
         case .notPermitted: self = .EPERM
         case .noSuchFileOrDirectory: self = .ENOENT
diff --git a/Sources/WASI/Platform/SandboxPrimitives/Open.swift b/Sources/WASI/Platform/SandboxPrimitives/Open.swift
@@ -1,6 +1,21 @@
 import SystemExtras
 import SystemPackage
 
+#if canImport(Darwin)
+    import Darwin
+#elseif canImport(Glibc)
+    import CSystem
+    import Glibc
+#elseif canImport(Musl)
+    import CSystem
+    import Musl
+#elseif os(Windows)
+    import CSystem
+    import ucrt
+#else
+    #error("Unsupported Platform")
+#endif
+
 struct PathResolution {
     private let mode: FileDescriptor.AccessMode
     private let options: FileDescriptor.OpenOptions
@@ -10,7 +25,20 @@ struct PathResolution {
     private let path: FilePath
     private var openDirectories: [FileDescriptor]
     /// Reverse-ordered remaining path components
+    /// File name appears first, then parent directories.
+    ///   e.g. `a/b/c` -> ["c", "b", "a"]
+    /// This ordering is just to avoid dropFirst() on Array.
     private var components: FilePath.ComponentView
+    private var resolvedSymlinks: Int = 0
+
+    private static var MAX_SYMLINKS: Int {
+        // Linux defines MAXSYMLINKS as 40, but on darwin platforms, it's 32.
+        // Take a single conservative value here to avoid platform-specific
+        // behavior as much as possible.
+        // * https://github.com/apple-oss-distributions/xnu/blob/8d741a5de7ff4191bf97d57b9f54c2f6d4a15585/bsd/sys/param.h#L207
+        // * https://github.com/torvalds/linux/blob/850925a8133c73c4a2453c360b2c3beb3bab67c9/include/linux/namei.h#L13
+        return 32
+    }
 
     init(
         baseDirFd: FileDescriptor,
@@ -33,39 +61,107 @@ struct PathResolution {
             // no more parent directory means too many `..`
             throw WASIAbi.Errno.EPERM
         }
+        try self.baseFd.close()
         self.baseFd = lastDirectory
     }
 
     mutating func regular(component: FilePath.Component) throws {
-        let options: FileDescriptor.OpenOptions
+        var options: FileDescriptor.OpenOptions = []
+        #if !os(Windows)
+            // First, try without following symlinks as a fast path.
+            // If it's actually a symlink and options don't have O_NOFOLLOW,
+            // we'll try again with interpreting resolved symlink.
+            options.insert(.noFollow)
+        #endif
         let mode: FileDescriptor.AccessMode
-        if !self.components.isEmpty {
-            var intermediateOptions: FileDescriptor.OpenOptions = []
 
+        if !self.components.isEmpty {
             #if !os(Windows)
                 // When trying to open an intermediate directory,
                 // we can assume it's directory.
-                intermediateOptions.insert(.directory)
-                // FIXME: Resolve symlink in safe way
-                intermediateOptions.insert(.noFollow)
+                options.insert(.directory)
             #endif
-            options = intermediateOptions
             mode = .readOnly
         } else {
-            options = self.options
+            options.formUnion(self.options)
             mode = self.mode
         }
 
         try WASIAbi.Errno.translatingPlatformErrno {
-            let newFd = try self.baseFd.open(
-                at: FilePath(root: nil, components: component),
-                mode, options: options, permissions: permissions
-            )
-            self.openDirectories.append(self.baseFd)
-            self.baseFd = newFd
+            do {
+                let newFd = try self.baseFd.open(
+                    at: FilePath(root: nil, components: component),
+                    mode, options: options, permissions: permissions
+                )
+                self.openDirectories.append(self.baseFd)
+                self.baseFd = newFd
+                return
+            } catch let openErrno as Errno {
+                if self.options.contains(.noFollow) {
+                    // If "open" failed with O_NOFOLLOW, no need to retry.
+                    throw openErrno
+                }
+
+                // If "open" failed and it might be a symlink, try again with following symlink.
+
+                // Check if it's a symlink by fstatat(2).
+                //
+                // NOTE: `errno` has enough information to check if the component is a symlink,
+                // but the value is platform-specific (e.g. ELOOP on POSIX standards, but EMLINK
+                // on BSD family), so we conservatively check it by fstatat(2).
+                let attrs = try self.baseFd.attributes(
+                    at: FilePath(root: nil, components: component), options: [.noFollow]
+                )
+                guard attrs.fileType.isSymlink else {
+                    // openat(2) failed, fstatat(2) succeeded, and it said it's not a symlink.
+                    // If it's not a symlink, the error is not due to symlink following
+                    // but other reasons, so just throw the error.
+                    // e.g. open with O_DIRECTORY on a regular file.
+                    throw openErrno
+                }
+
+                try self.symlink(component: component)
+            }
         }
     }
 
+    mutating func symlink(component: FilePath.Component) throws {
+        /// Thin wrapper around readlinkat(2)
+        func _readlinkat(_ fd: CInt, _ path: UnsafePointer<CChar>) throws -> FilePath {
+            var buffer = [CChar](repeating: 0, count: Int(PATH_MAX))
+            let length = buffer.withUnsafeMutableBufferPointer { buffer in
+                buffer.withMemoryRebound(to: Int8.self) { buffer in
+                    readlinkat(fd, path, buffer.baseAddress, buffer.count)
+                }
+            }
+            guard length >= 0 else {
+                throw try WASIAbi.Errno(platformErrno: errno)
+            }
+            return FilePath(String(cString: buffer))
+        }
+
+        guard resolvedSymlinks < Self.MAX_SYMLINKS else {
+            throw WASIAbi.Errno.ELOOP
+        }
+
+        // If it's a symlink, readlink(2) and check it doesn't escape sandbox.
+        let linkPath = try component.withPlatformString {
+            return try _readlinkat(self.baseFd.rawValue, $0)
+        }
+
+        guard !linkPath.isAbsolute else {
+            // Ban absolute symlink to avoid sandbox-escaping.
+            throw WASIAbi.Errno.EPERM
+        }
+
+        // Increment the number of resolved symlinks to prevent infinite
+        // link loop.
+        resolvedSymlinks += 1
+
+        // Add resolved path to the worklist.
+        self.components.append(contentsOf: linkPath.components.reversed())
+    }
+
     mutating func resolve() throws -> FileDescriptor {
         if path.isAbsolute {
             // POSIX openat(2) interprets absolute path ignoring base directory fd
diff --git a/Tests/WASITests/TestSupport.swift b/Tests/WASITests/TestSupport.swift
@@ -0,0 +1,64 @@
+import Foundation
+
+enum TestSupport {
+    struct Error: Swift.Error, CustomStringConvertible {
+        let description: String
+
+        init(description: String) {
+            self.description = description
+        }
+
+        init(errno: Int32) {
+            self.init(description: String(cString: strerror(errno)))
+        }
+    }
+
+    class TemporaryDirectory {
+        let path: String
+        var url: URL { URL(fileURLWithPath: path) }
+
+        init() throws {
+            let tempdir = URL(fileURLWithPath: NSTemporaryDirectory())
+            let templatePath = tempdir.appendingPathComponent("WasmKit.XXXXXX")
+            var template = [UInt8](templatePath.path.utf8).map({ Int8($0) }) + [Int8(0)]
+
+            #if os(Windows)
+                if _mktemp_s(&template, template.count) != 0 {
+                    throw Error(errno: errno)
+                }
+                if _mkdir(template) != 0 {
+                    throw Error(errno: errno)
+                }
+            #else
+                if mkdtemp(&template) == nil {
+                    throw Error(errno: errno)
+                }
+            #endif
+
+            self.path = String(cString: template)
+        }
+
+        func createDir(at relativePath: String) throws {
+            let directoryURL = url.appendingPathComponent(relativePath)
+            try FileManager.default.createDirectory(at: directoryURL, withIntermediateDirectories: true, attributes: nil)
+        }
+
+        func createFile(at relativePath: String, contents: String) throws {
+            let fileURL = url.appendingPathComponent(relativePath)
+            guard let data = contents.data(using: .utf8) else { return }
+            FileManager.default.createFile(atPath: fileURL.path, contents: data, attributes: nil)
+        }
+
+        func createSymlink(at relativePath: String, to target: String) throws {
+            let linkURL = url.appendingPathComponent(relativePath)
+            try FileManager.default.createSymbolicLink(
+                atPath: linkURL.path,
+                withDestinationPath: target
+            )
+        }
+
+        deinit {
+            _ = try? FileManager.default.removeItem(atPath: path)
+        }
+    }
+}
diff --git a/Tests/WASITests/WASITests.swift b/Tests/WASITests/WASITests.swift
@@ -0,0 +1,134 @@
+import XCTest
+
+@testable import WASI
+
+final class WASITests: XCTestCase {
+    func testPathOpen() throws {
+        let t = try TestSupport.TemporaryDirectory()
+
+        try t.createDir(at: "External")
+        try t.createDir(at: "External/secret-dir-b")
+        try t.createFile(at: "External/secret-a.txt", contents: "Secret A")
+        try t.createFile(at: "External/secret-dir-b/secret-c.txt", contents: "Secret C")
+        try t.createDir(at: "Sandbox")
+        try t.createFile(at: "Sandbox/hello.txt", contents: "Hello")
+        try t.createSymlink(at: "Sandbox/link-hello.txt", to: "hello.txt")
+        try t.createDir(at: "Sandbox/world.dir")
+        try t.createSymlink(at: "Sandbox/link-world.dir", to: "world.dir")
+        try t.createSymlink(at: "Sandbox/link-external-secret-a.txt", to: "../External/secret-a.txt")
+        try t.createSymlink(at: "Sandbox/link-secret-dir-b", to: "../External/secret-dir-b")
+        try t.createSymlink(at: "Sandbox/link-updown-hello.txt", to: "../Sandbox/link-updown-hello.txt")
+        try t.createSymlink(at: "Sandbox/link-external-non-existent.txt", to: "../External/non-existent.txt")
+        try t.createSymlink(at: "Sandbox/link-root", to: "/")
+        try t.createSymlink(at: "Sandbox/link-loop.txt", to: "link-loop.txt")
+
+        let wasi = try WASIBridgeToHost(
+            preopens: ["/Sandbox": t.url.appendingPathComponent("Sandbox").path]
+        )
+        let mntFd: WASIAbi.Fd = 3
+
+        func assertResolve(_ path: String, followSymlink: Bool, directory: Bool = false) throws {
+            let fd = try wasi.path_open(
+                dirFd: mntFd,
+                dirFlags: followSymlink ? [.SYMLINK_FOLLOW] : [],
+                path: path,
+                oflags: directory ? [.DIRECTORY] : [],
+                fsRightsBase: .DIRECTORY_BASE_RIGHTS,
+                fsRightsInheriting: .DIRECTORY_INHERITING_RIGHTS,
+                fdflags: []
+            )
+            try wasi.fd_close(fd: fd)
+        }
+
+        func assertNotResolve(
+            _ path: String,
+            followSymlink: Bool,
+            directory: Bool = false,
+            file: StaticString = #file,
+            line: UInt = #line,
+            _ checkError: ((WASIAbi.Errno) throws -> Void)?
+        ) throws {
+            do {
+                _ = try wasi.path_open(
+                    dirFd: mntFd,
+                    dirFlags: followSymlink ? [.SYMLINK_FOLLOW] : [],
+                    path: path,
+                    oflags: directory ? [.DIRECTORY] : [],
+                    fsRightsBase: .DIRECTORY_BASE_RIGHTS,
+                    fsRightsInheriting: .DIRECTORY_INHERITING_RIGHTS,
+                    fdflags: []
+                )
+                XCTFail("Expected not to be able to open \(path)", file: file, line: line)
+            } catch {
+                guard let error = error as? WASIAbi.Errno else {
+                    XCTFail("Expected WASIAbi.Errno error but got \(error)", file: file, line: line)
+                    return
+                }
+                try checkError?(error)
+            }
+        }
+
+        try assertNotResolve("non-existent.txt", followSymlink: false) { error in
+            XCTAssertEqual(error, .ENOENT)
+        }
+
+        try assertResolve("link-hello.txt", followSymlink: true)
+        try assertNotResolve("link-hello.txt", followSymlink: false) { error in
+            XCTAssertEqual(error, .ELOOP)
+        }
+        try assertNotResolve("link-hello.txt", followSymlink: true, directory: true) { error in
+            XCTAssertEqual(error, .ENOTDIR)
+        }
+
+        try assertNotResolve("link-hello.txt/", followSymlink: true) { error in
+            XCTAssertEqual(error, .ENOTDIR)
+        }
+
+        try assertResolve("link-world.dir", followSymlink: true)
+        try assertNotResolve("link-world.dir", followSymlink: false) { error in
+            XCTAssertEqual(error, .ELOOP)
+        }
+
+        try assertNotResolve("link-external-secret-a.txt", followSymlink: true) { error in
+            XCTAssertEqual(error, .EPERM)
+        }
+        try assertNotResolve("link-external-secret-a.txt", followSymlink: false) { error in
+            XCTAssertEqual(error, .ELOOP)
+        }
+
+        try assertNotResolve("link-external-non-existent.txt", followSymlink: true) { error in
+            XCTAssertEqual(error, .EPERM)
+        }
+        try assertNotResolve("link-external-non-existent.txt", followSymlink: false) { error in
+            XCTAssertEqual(error, .ELOOP)
+        }
+
+        try assertNotResolve("link-updown-hello.txt", followSymlink: true) { error in
+            XCTAssertEqual(error, .EPERM)
+        }
+        try assertNotResolve("link-updown-hello.txt", followSymlink: false) { error in
+            XCTAssertEqual(error, .ELOOP)
+        }
+
+        try assertNotResolve("link-secret-dir-b/secret-c.txt", followSymlink: true) { error in
+            XCTAssertEqual(error, .EPERM)
+        }
+        try assertNotResolve("link-secret-dir-b/secret-c.txt", followSymlink: false) { error in
+            XCTAssertEqual(error, .ENOTDIR)
+        }
+
+        try assertNotResolve("link-root", followSymlink: true) { error in
+            XCTAssertEqual(error, .EPERM)
+        }
+        try assertNotResolve("link-root", followSymlink: false) { error in
+            XCTAssertEqual(error, .ELOOP)
+        }
+
+        try assertNotResolve("link-loop.txt", followSymlink: false) { error in
+            XCTAssertEqual(error, .ELOOP)
+        }
+        try assertNotResolve("link-loop.txt", followSymlink: true) { error in
+            XCTAssertEqual(error, .ELOOP)
+        }
+    }
+}
