From 9e965cb684dc2b99f5782062f84fdf23eb9194f3 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Sun, 27 Oct 2024 01:43:20 +0900 Subject: [PATCH] WASI: Implement `path_open` with proper symlink resolution --- Sources/WASI/Platform/PlatformTypes.swift | 20 ++- .../Platform/SandboxPrimitives/Open.swift | 134 +++++++++++++++-- Tests/WASITests/TestSupport.swift | 64 ++++++++ Tests/WASITests/WASITests.swift | 137 ++++++++++++++++++ 4 files changed, 335 insertions(+), 20 deletions(-) create mode 100644 Tests/WASITests/TestSupport.swift create mode 100644 Tests/WASITests/WASITests.swift diff --git a/Sources/WASI/Platform/PlatformTypes.swift b/Sources/WASI/Platform/PlatformTypes.swift index aeae09b4..6b0637ac 100644 --- 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 index c6d4c293..651331ad 100644 --- 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,117 @@ 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 os(Windows) + // Windows doesn't have O_NOFOLLOW, so we can't retry with following symlink. + throw openErrno + #else + 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) + #endif + } } } + #if !os(Windows) + mutating func symlink(component: FilePath.Component) throws { + /// Thin wrapper around readlinkat(2) + func _readlinkat(_ fd: CInt, _ path: UnsafePointer) throws -> FilePath { + var buffer = [CChar](repeating: 0, count: Int(PATH_MAX)) + let length = try buffer.withUnsafeMutableBufferPointer { buffer in + try buffer.withMemoryRebound(to: Int8.self) { buffer in + guard let bufferBase = buffer.baseAddress else { + throw WASIAbi.Errno.EINVAL + } + return readlinkat(fd, path, bufferBase, 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()) + } + #endif + 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 new file mode 100644 index 00000000..6059cb34 --- /dev/null +++ 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 new file mode 100644 index 00000000..135f122f --- /dev/null +++ b/Tests/WASITests/WASITests.swift @@ -0,0 +1,137 @@ +import XCTest + +@testable import WASI + +final class WASITests: XCTestCase { + func testPathOpen() throws { + #if os(Windows) + try XCTSkipIf(true) + #endif + 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) + } + } +}