From 1c1969d438cb9223c00647fe47f148cfd40cb2ce Mon Sep 17 00:00:00 2001 From: Jeremy Schonfeld Date: Tue, 22 Oct 2024 14:13:04 -0700 Subject: [PATCH 1/3] Avoid racy stdlib functions for fetching user/group info --- .../FileManager/FileManager+Files.swift | 24 ++----- .../FileManager/FileManager+Utilities.swift | 14 ---- .../FileManager+DarwinSearchPaths.swift | 17 +---- Sources/FoundationEssentials/Platform.swift | 64 +++++++++++++++++++ .../ProcessInfo/ProcessInfo.swift | 10 ++- .../String/String+Path.swift | 13 ++-- .../StringTests.swift | 11 ---- 7 files changed, 81 insertions(+), 72 deletions(-) diff --git a/Sources/FoundationEssentials/FileManager/FileManager+Files.swift b/Sources/FoundationEssentials/FileManager/FileManager+Files.swift index 6e1799506..592b27800 100644 --- a/Sources/FoundationEssentials/FileManager/FileManager+Files.swift +++ b/Sources/FoundationEssentials/FileManager/FileManager+Files.swift @@ -41,22 +41,6 @@ extension Date { } #if !os(Windows) -#if !os(WASI) -private func _nameFor(uid: uid_t) -> String? { - guard let pwd = getpwuid(uid), let name = pwd.pointee.pw_name else { - return nil - } - return String(cString: name) -} - -private func _nameFor(gid: gid_t) -> String? { - guard let pwd = getgrgid(gid), let name = pwd.pointee.gr_name else { - return nil - } - return String(cString: name) -} -#endif - extension mode_t { private var _fileType: FileAttributeType { switch self & S_IFMT { @@ -193,10 +177,10 @@ extension stat { .groupOwnerAccountID : _writeFileAttributePrimitive(st_gid, as: UInt.self) ] #if !os(WASI) - if let userName = _nameFor(uid: st_uid) { + if let userName = Platform.nameFor(uid: st_uid) { result[.ownerAccountName] = userName } - if let groupName = _nameFor(gid: st_gid) { + if let groupName = Platform.nameFor(gid: st_gid) { result[.groupOwnerAccountName] = groupName } #endif @@ -942,8 +926,8 @@ extension _FileManagerImpl { #else // Bias toward userID & groupID - try to prevent round trips to getpwnam if possible. var leaveUnchanged: UInt32 { UInt32(bitPattern: -1) } - let rawUserID = userID.flatMap(uid_t.init) ?? user.flatMap(Self._userAccountNameToNumber) ?? leaveUnchanged - let rawGroupID = groupID.flatMap(gid_t.init) ?? group.flatMap(Self._groupAccountNameToNumber) ?? leaveUnchanged + let rawUserID = userID.flatMap(uid_t.init) ?? user.flatMap(Platform.uidFor(name:)) ?? leaveUnchanged + let rawGroupID = groupID.flatMap(gid_t.init) ?? group.flatMap(Platform.gidFor(name:)) ?? leaveUnchanged if chown(fileSystemRepresentation, rawUserID, rawGroupID) != 0 { throw CocoaError.errorWithFilePath(path, errno: errno, reading: false) } diff --git a/Sources/FoundationEssentials/FileManager/FileManager+Utilities.swift b/Sources/FoundationEssentials/FileManager/FileManager+Utilities.swift index fac053926..0d34680ae 100644 --- a/Sources/FoundationEssentials/FileManager/FileManager+Utilities.swift +++ b/Sources/FoundationEssentials/FileManager/FileManager+Utilities.swift @@ -275,20 +275,6 @@ extension _FileManagerImpl { } } #endif - -#if !os(Windows) && !os(WASI) - static func _userAccountNameToNumber(_ name: String) -> uid_t? { - name.withCString { ptr in - getpwnam(ptr)?.pointee.pw_uid - } - } - - static func _groupAccountNameToNumber(_ name: String) -> gid_t? { - name.withCString { ptr in - getgrnam(ptr)?.pointee.gr_gid - } - } -#endif } extension FileManager { diff --git a/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift b/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift index f34d1bfe5..c1cdf4487 100644 --- a/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift +++ b/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift @@ -160,20 +160,9 @@ extension String { guard self == "~" || self.hasPrefix("~/") else { return self } - var bufSize = sysconf(_SC_GETPW_R_SIZE_MAX) - if bufSize == -1 { - bufSize = 4096 // A generous guess. - } - return withUnsafeTemporaryAllocation(of: CChar.self, capacity: bufSize) { pwBuff in - var pw: UnsafeMutablePointer? - var pwd = passwd() - let euid = geteuid() - let trueUid = euid == 0 ? getuid() : euid - guard getpwuid_r(trueUid, &pwd, pwBuff.baseAddress!, bufSize, &pw) == 0, let pw else { - return self - } - return String(cString: pw.pointee.pw_dir).appendingPathComponent(String(self.dropFirst())) - } + let euid = geteuid() + let trueUid = euid == 0 ? getuid() : euid + return Platform.nameFor(uid: trueUid).appendingPathComponent(String(self.dropFirst())) } } #endif // os(macOS) && FOUNDATION_FRAMEWORK diff --git a/Sources/FoundationEssentials/Platform.swift b/Sources/FoundationEssentials/Platform.swift index 0319693d5..433b0cf57 100644 --- a/Sources/FoundationEssentials/Platform.swift +++ b/Sources/FoundationEssentials/Platform.swift @@ -135,6 +135,70 @@ extension Platform { } return result } + + #if canImport(Darwin) + typealias Operation = (Input, UnsafeMutablePointer?, UnsafeMutablePointer?, Int, UnsafeMutablePointer?>?) -> Int32 + #else + typealias Operation = (Input, UnsafeMutablePointer, UnsafeMutablePointer, Int, UnsafeMutablePointer?>) -> Int32 + #endif + + private static func withUserGroupBuffer(_ input: Input, _ output: Output, sizeProperty: Int32, operation: Operation, block: (Output) throws -> R) rethrows -> R? { + var bufferLen = sysconf(sizeProperty) + if bufferLen == -1 { + bufferLen = 4096 // Generous default size estimate + } + return try withUnsafeTemporaryAllocation(of: CChar.self, capacity: bufferLen) { + var result = output + var ptr: UnsafeMutablePointer? + let error = operation(input, &result, $0.baseAddress!, bufferLen, &ptr) + guard error == 0 && ptr != nil else { + return nil + } + return try block(result) + } + } + + static func uidFor(name: String) -> uid_t? { + withUserGroupBuffer(name, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwnam_r) { + $0.pw_uid + } + } + + static func gidFor(name: String) -> uid_t? { + withUserGroupBuffer(name, group(), sizeProperty: Int32(_SC_GETGR_R_SIZE_MAX), operation: getgrnam_r) { + $0.gr_gid + } + } + + static func nameFor(uid: uid_t) -> String? { + withUserGroupBuffer(uid, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwuid_r) { + String(cString: $0.pw_name) + } + } + + static func fullNameFor(uid: uid_t) -> String? { + withUserGroupBuffer(uid, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwuid_r) { + String(cString: $0.pw_gecos) + } + } + + static func nameFor(gid: gid_t) -> String? { + withUserGroupBuffer(gid, group(), sizeProperty: Int32(_SC_GETGR_R_SIZE_MAX), operation: getgrgid_r) { + String(cString: $0.gr_name) + } + } + + static func homeDirFor(name: String) -> String? { + withUserGroupBuffer(name, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwnam_r) { + String(cString: $0.pw_dir) + } + } + + static func homeDirFor(uid: uid_t) -> String? { + withUserGroupBuffer(uid, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwuid_r) { + String(cString: $0.pw_dir) + } + } } #endif diff --git a/Sources/FoundationEssentials/ProcessInfo/ProcessInfo.swift b/Sources/FoundationEssentials/ProcessInfo/ProcessInfo.swift index 0729e91c6..254a76c18 100644 --- a/Sources/FoundationEssentials/ProcessInfo/ProcessInfo.swift +++ b/Sources/FoundationEssentials/ProcessInfo/ProcessInfo.swift @@ -178,9 +178,8 @@ final class _ProcessInfo: Sendable { #if canImport(Darwin) || canImport(Android) || canImport(Glibc) || canImport(Musl) // Darwin and Linux let (euid, _) = Platform.getUGIDs() - if let upwd = getpwuid(euid), - let uname = upwd.pointee.pw_name { - return String(cString: uname) + if let username = Platform.nameFor(uid: euid) { + return username } else if let username = self.environment["USER"] { return username } @@ -215,9 +214,8 @@ final class _ProcessInfo: Sendable { return "" #elseif canImport(Darwin) || canImport(Android) || canImport(Glibc) || canImport(Musl) let (euid, _) = Platform.getUGIDs() - if let upwd = getpwuid(euid), - let fullname = upwd.pointee.pw_gecos { - return String(cString: fullname) + if let fullName = Platform.fullNameFor(uid: euid) { + return fullName } return "" #elseif os(WASI) diff --git a/Sources/FoundationEssentials/String/String+Path.swift b/Sources/FoundationEssentials/String/String+Path.swift index 636cc54d0..fece52aca 100644 --- a/Sources/FoundationEssentials/String/String+Path.swift +++ b/Sources/FoundationEssentials/String/String+Path.swift @@ -513,17 +513,16 @@ extension String { #if !os(WASI) // WASI does not have user concept // Next, attempt to find the home directory via getpwnam/getpwuid - var pass: UnsafeMutablePointer? if let user { - pass = getpwnam(user) + if let dir = Platform.homeDirFor(name: user) { + return dir.standardizingPath + } } else { // We use the real UID instead of the EUID here when the EUID is the root user (i.e. a process has called seteuid(0)) // In this instance, we historically do this to ensure a stable home directory location for processes that call seteuid(0) - pass = getpwuid(Platform.getUGIDs(allowEffectiveRootUID: false).uid) - } - - if let dir = pass?.pointee.pw_dir { - return String(cString: dir).standardizingPath + if let dir = Platform.homeDirFor(uid: Platform.getUGIDs(allowEffectiveRootUID: false).uid) { + return dir.standardizingPath + } } #endif // !os(WASI) diff --git a/Tests/FoundationEssentialsTests/StringTests.swift b/Tests/FoundationEssentialsTests/StringTests.swift index 54d9e7bb9..1daa98b92 100644 --- a/Tests/FoundationEssentialsTests/StringTests.swift +++ b/Tests/FoundationEssentialsTests/StringTests.swift @@ -2576,17 +2576,6 @@ final class StringTestsStdlib: XCTestCase { expectTrue(availableEncodings.contains("abc".smallestEncoding)) } - func getHomeDir() -> String { -#if os(macOS) - return String(cString: getpwuid(getuid()).pointee.pw_dir) -#elseif canImport(Darwin) - // getpwuid() returns null in sandboxed apps under iOS simulator. - return NSHomeDirectory() -#else - preconditionFailed("implement") -#endif - } - func test_addingPercentEncoding() { expectEqual( "abcd1234", From 6000a674916ce07882702bb4c0c358f23dff0708 Mon Sep 17 00:00:00 2001 From: Jeremy Schonfeld Date: Tue, 22 Oct 2024 15:02:42 -0700 Subject: [PATCH 2/3] Refactor naming --- .../FileManager/FileManager+Files.swift | 8 ++++---- .../FileManager+DarwinSearchPaths.swift | 2 +- Sources/FoundationEssentials/Platform.swift | 16 ++++++++-------- .../ProcessInfo/ProcessInfo.swift | 4 ++-- .../String/String+Path.swift | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Sources/FoundationEssentials/FileManager/FileManager+Files.swift b/Sources/FoundationEssentials/FileManager/FileManager+Files.swift index 592b27800..7a0b35e8b 100644 --- a/Sources/FoundationEssentials/FileManager/FileManager+Files.swift +++ b/Sources/FoundationEssentials/FileManager/FileManager+Files.swift @@ -177,10 +177,10 @@ extension stat { .groupOwnerAccountID : _writeFileAttributePrimitive(st_gid, as: UInt.self) ] #if !os(WASI) - if let userName = Platform.nameFor(uid: st_uid) { + if let userName = Platform.name(forUID: st_uid) { result[.ownerAccountName] = userName } - if let groupName = Platform.nameFor(gid: st_gid) { + if let groupName = Platform.name(forGID: st_gid) { result[.groupOwnerAccountName] = groupName } #endif @@ -926,8 +926,8 @@ extension _FileManagerImpl { #else // Bias toward userID & groupID - try to prevent round trips to getpwnam if possible. var leaveUnchanged: UInt32 { UInt32(bitPattern: -1) } - let rawUserID = userID.flatMap(uid_t.init) ?? user.flatMap(Platform.uidFor(name:)) ?? leaveUnchanged - let rawGroupID = groupID.flatMap(gid_t.init) ?? group.flatMap(Platform.gidFor(name:)) ?? leaveUnchanged + let rawUserID = userID.flatMap(uid_t.init) ?? user.flatMap(Platform.uid(forName:)) ?? leaveUnchanged + let rawGroupID = groupID.flatMap(gid_t.init) ?? group.flatMap(Platform.gid(forName:)) ?? leaveUnchanged if chown(fileSystemRepresentation, rawUserID, rawGroupID) != 0 { throw CocoaError.errorWithFilePath(path, errno: errno, reading: false) } diff --git a/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift b/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift index c1cdf4487..e1d3f6010 100644 --- a/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift +++ b/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift @@ -162,7 +162,7 @@ extension String { } let euid = geteuid() let trueUid = euid == 0 ? getuid() : euid - return Platform.nameFor(uid: trueUid).appendingPathComponent(String(self.dropFirst())) + return Platform.name(forUID: trueUid).appendingPathComponent(String(self.dropFirst())) } } #endif // os(macOS) && FOUNDATION_FRAMEWORK diff --git a/Sources/FoundationEssentials/Platform.swift b/Sources/FoundationEssentials/Platform.swift index 433b0cf57..d8b8fe6f0 100644 --- a/Sources/FoundationEssentials/Platform.swift +++ b/Sources/FoundationEssentials/Platform.swift @@ -158,43 +158,43 @@ extension Platform { } } - static func uidFor(name: String) -> uid_t? { + static func uid(forName name: String) -> uid_t? { withUserGroupBuffer(name, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwnam_r) { $0.pw_uid } } - static func gidFor(name: String) -> uid_t? { + static func gid(forName name: String) -> uid_t? { withUserGroupBuffer(name, group(), sizeProperty: Int32(_SC_GETGR_R_SIZE_MAX), operation: getgrnam_r) { $0.gr_gid } } - static func nameFor(uid: uid_t) -> String? { + static func name(forUID uid: uid_t) -> String? { withUserGroupBuffer(uid, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwuid_r) { String(cString: $0.pw_name) } } - static func fullNameFor(uid: uid_t) -> String? { + static func fullName(forUID uid: uid_t) -> String? { withUserGroupBuffer(uid, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwuid_r) { String(cString: $0.pw_gecos) } } - static func nameFor(gid: gid_t) -> String? { + static func name(forGID gid: gid_t) -> String? { withUserGroupBuffer(gid, group(), sizeProperty: Int32(_SC_GETGR_R_SIZE_MAX), operation: getgrgid_r) { String(cString: $0.gr_name) } } - static func homeDirFor(name: String) -> String? { - withUserGroupBuffer(name, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwnam_r) { + static func homeDirectory(forUserName userName: String) -> String? { + withUserGroupBuffer(userName, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwnam_r) { String(cString: $0.pw_dir) } } - static func homeDirFor(uid: uid_t) -> String? { + static func homeDirectory(forUID uid: uid_t) -> String? { withUserGroupBuffer(uid, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwuid_r) { String(cString: $0.pw_dir) } diff --git a/Sources/FoundationEssentials/ProcessInfo/ProcessInfo.swift b/Sources/FoundationEssentials/ProcessInfo/ProcessInfo.swift index 254a76c18..22f2ac068 100644 --- a/Sources/FoundationEssentials/ProcessInfo/ProcessInfo.swift +++ b/Sources/FoundationEssentials/ProcessInfo/ProcessInfo.swift @@ -178,7 +178,7 @@ final class _ProcessInfo: Sendable { #if canImport(Darwin) || canImport(Android) || canImport(Glibc) || canImport(Musl) // Darwin and Linux let (euid, _) = Platform.getUGIDs() - if let username = Platform.nameFor(uid: euid) { + if let username = Platform.name(forUID: euid) { return username } else if let username = self.environment["USER"] { return username @@ -214,7 +214,7 @@ final class _ProcessInfo: Sendable { return "" #elseif canImport(Darwin) || canImport(Android) || canImport(Glibc) || canImport(Musl) let (euid, _) = Platform.getUGIDs() - if let fullName = Platform.fullNameFor(uid: euid) { + if let fullName = Platform.fullName(forUID: euid) { return fullName } return "" diff --git a/Sources/FoundationEssentials/String/String+Path.swift b/Sources/FoundationEssentials/String/String+Path.swift index fece52aca..a2ebfe826 100644 --- a/Sources/FoundationEssentials/String/String+Path.swift +++ b/Sources/FoundationEssentials/String/String+Path.swift @@ -514,13 +514,13 @@ extension String { #if !os(WASI) // WASI does not have user concept // Next, attempt to find the home directory via getpwnam/getpwuid if let user { - if let dir = Platform.homeDirFor(name: user) { + if let dir = Platform.homeDirectory(forUserName: user) { return dir.standardizingPath } } else { // We use the real UID instead of the EUID here when the EUID is the root user (i.e. a process has called seteuid(0)) // In this instance, we historically do this to ensure a stable home directory location for processes that call seteuid(0) - if let dir = Platform.homeDirFor(uid: Platform.getUGIDs(allowEffectiveRootUID: false).uid) { + if let dir = Platform.homeDirectory(forUID: Platform.getUGIDs(allowEffectiveRootUID: false).uid) { return dir.standardizingPath } } From 146c72ca60c255b9ccfc72990c31f67d067b4967 Mon Sep 17 00:00:00 2001 From: Jeremy Schonfeld Date: Tue, 22 Oct 2024 17:27:44 -0700 Subject: [PATCH 3/3] Fix build failure --- .../SearchPaths/FileManager+DarwinSearchPaths.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift b/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift index e1d3f6010..8aa397f97 100644 --- a/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift +++ b/Sources/FoundationEssentials/FileManager/SearchPaths/FileManager+DarwinSearchPaths.swift @@ -162,7 +162,10 @@ extension String { } let euid = geteuid() let trueUid = euid == 0 ? getuid() : euid - return Platform.name(forUID: trueUid).appendingPathComponent(String(self.dropFirst())) + guard let name = Platform.name(forUID: trueUid) else { + return self + } + return name.appendingPathComponent(String(self.dropFirst())) } } #endif // os(macOS) && FOUNDATION_FRAMEWORK