Skip to content

Fetching user/group info causes race conditions #994

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.

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

Merged
merged 3 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 20 deletions Sources/FoundationEssentials/FileManager/FileManager+Files.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.name(forUID: st_uid) {
result[.ownerAccountName] = userName
}
if let groupName = _nameFor(gid: st_gid) {
if let groupName = Platform.name(forGID: st_gid) {
result[.groupOwnerAccountName] = groupName
}
#endif
Expand Down Expand Up @@ -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.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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,20 +160,12 @@ 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<passwd>?
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
guard let name = Platform.name(forUID: trueUid) else {
return self
}
return name.appendingPathComponent(String(self.dropFirst()))
}
}
#endif // os(macOS) && FOUNDATION_FRAMEWORK
Expand Down
64 changes: 64 additions & 0 deletions Sources/FoundationEssentials/Platform.swift
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,70 @@ extension Platform {
}
return result
}

#if canImport(Darwin)
typealias Operation<Input, Output> = (Input, UnsafeMutablePointer<Output>?, UnsafeMutablePointer<CChar>?, Int, UnsafeMutablePointer<UnsafeMutablePointer<Output>?>?) -> Int32
#else
typealias Operation<Input, Output> = (Input, UnsafeMutablePointer<Output>, UnsafeMutablePointer<CChar>, Int, UnsafeMutablePointer<UnsafeMutablePointer<Output>?>) -> Int32
#endif

private static func withUserGroupBuffer<Input, Output, R>(_ input: Input, _ output: Output, sizeProperty: Int32, operation: Operation<Input, Output>, 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<Output>?
let error = operation(input, &result, $0.baseAddress!, bufferLen, &ptr)
guard error == 0 && ptr != nil else {
return nil
}
return try block(result)
}
}

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 gid(forName name: String) -> uid_t? {
withUserGroupBuffer(name, group(), sizeProperty: Int32(_SC_GETGR_R_SIZE_MAX), operation: getgrnam_r) {
$0.gr_gid
}
}

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 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 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 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 homeDirectory(forUID uid: uid_t) -> String? {
withUserGroupBuffer(uid, passwd(), sizeProperty: Int32(_SC_GETPW_R_SIZE_MAX), operation: getpwuid_r) {
String(cString: $0.pw_dir)
}
}
}
#endif

Expand Down
10 changes: 4 additions & 6 deletions Sources/FoundationEssentials/ProcessInfo/ProcessInfo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.name(forUID: euid) {
return username
} else if let username = self.environment["USER"] {
return username
}
Expand Down Expand Up @@ -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.fullName(forUID: euid) {
return fullName
}
return ""
#elseif os(WASI)
Expand Down
13 changes: 6 additions & 7 deletions Sources/FoundationEssentials/String/String+Path.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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<passwd>?
if let user {
pass = getpwnam(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)
pass = getpwuid(Platform.getUGIDs(allowEffectiveRootUID: false).uid)
}

if let dir = pass?.pointee.pw_dir {
return String(cString: dir).standardizingPath
if let dir = Platform.homeDirectory(forUID: Platform.getUGIDs(allowEffectiveRootUID: false).uid) {
return dir.standardizingPath
}
}
#endif // !os(WASI)

Expand Down
11 changes: 0 additions & 11 deletions Tests/FoundationEssentialsTests/StringTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down