Fetching user/group info causes race conditions (#994)

jmschonfeld · web-flow · commit 71eefee7fca0 · 2024-10-23T09:18:15.000-07:00
* Avoid racy stdlib functions for fetching user/group info

* Refactor naming

* Fix build failure
diff --git 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.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
@@ -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)
                 }
diff --git 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
@@ -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
diff --git a/Sources/FoundationEssentials/Platform.swift b/Sources/FoundationEssentials/Platform.swift
@@ -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
 
diff --git 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.name(forUID: 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.fullName(forUID: euid) {
+            return fullName
         }
         return ""
 #elseif os(WASI)
diff --git 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<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)
 
diff --git 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",
