From 201c7737a6d3f74cc103f0d31880aaed0e381afb Mon Sep 17 00:00:00 2001 From: hariel1985 Date: Sat, 31 Jan 2026 16:49:40 +0100 Subject: [PATCH] Add PID validation to prevent TOCTOU race condition in process control Security improvements: - Validate process still exists before sending signals - Verify process start time matches to detect PID reuse - Add ProcessControlError enum with descriptive error messages - Show error alerts when operations fail (permission denied, process not found, PID reused) - Pass expectedStartTime to all process control operations This prevents accidentally terminating the wrong process when a PID gets reused between user selection and confirmation. Co-Authored-By: Claude Opus 4.5 --- TopManager/Services/SystemMonitor.swift | 130 +++++++++++++++++-- TopManager/Views/Processes/ProcessView.swift | 56 +++++++- 2 files changed, 166 insertions(+), 20 deletions(-) diff --git a/TopManager/Services/SystemMonitor.swift b/TopManager/Services/SystemMonitor.swift index 50742d9..7544263 100644 --- a/TopManager/Services/SystemMonitor.swift +++ b/TopManager/Services/SystemMonitor.swift @@ -179,24 +179,128 @@ final class SystemMonitor: ObservableObject { } } - // Process control methods - @discardableResult - func terminateProcess(_ pid: pid_t) -> Bool { - kill(pid, SIGTERM) == 0 + // Process control methods with PID validation + + /// Validates that a PID still refers to the same process before sending a signal + private func validateProcess(pid: pid_t, expectedStartTime: Date?) -> ProcessControlError? { + var bsdInfo = proc_bsdinfo() + let bsdInfoSize = Int32(MemoryLayout.size) + let result = proc_pidinfo(pid, PROC_PIDTBSDINFO, 0, &bsdInfo, bsdInfoSize) + + // Check if process still exists + guard result == bsdInfoSize else { + return .processNotFound + } + + // Validate start time matches (detects PID reuse) + if let expected = expectedStartTime { + let currentStartTime = Date(timeIntervalSince1970: TimeInterval(bsdInfo.pbi_start_tvsec)) + // Allow 1 second tolerance for timing differences + if abs(currentStartTime.timeIntervalSince(expected)) > 1.0 { + return .processChanged + } + } + + return nil // Validation passed } - @discardableResult - func forceKillProcess(_ pid: pid_t) -> Bool { - kill(pid, SIGKILL) == 0 + func terminateProcess(_ pid: pid_t, expectedStartTime: Date? = nil) -> Result { + // Validate PID still refers to same process + if let error = validateProcess(pid: pid, expectedStartTime: expectedStartTime) { + return .failure(error) + } + + let result = kill(pid, SIGTERM) + if result == 0 { + return .success(()) + } else { + return .failure(ProcessControlError.fromErrno(errno)) + } } - @discardableResult - func suspendProcess(_ pid: pid_t) -> Bool { - kill(pid, SIGSTOP) == 0 + func forceKillProcess(_ pid: pid_t, expectedStartTime: Date? = nil) -> Result { + // Validate PID still refers to same process + if let error = validateProcess(pid: pid, expectedStartTime: expectedStartTime) { + return .failure(error) + } + + let result = kill(pid, SIGKILL) + if result == 0 { + return .success(()) + } else { + return .failure(ProcessControlError.fromErrno(errno)) + } } - @discardableResult - func resumeProcess(_ pid: pid_t) -> Bool { - kill(pid, SIGCONT) == 0 + func suspendProcess(_ pid: pid_t, expectedStartTime: Date? = nil) -> Result { + if let error = validateProcess(pid: pid, expectedStartTime: expectedStartTime) { + return .failure(error) + } + + let result = kill(pid, SIGSTOP) + if result == 0 { + return .success(()) + } else { + return .failure(ProcessControlError.fromErrno(errno)) + } + } + + func resumeProcess(_ pid: pid_t, expectedStartTime: Date? = nil) -> Result { + if let error = validateProcess(pid: pid, expectedStartTime: expectedStartTime) { + return .failure(error) + } + + let result = kill(pid, SIGCONT) + if result == 0 { + return .success(()) + } else { + return .failure(ProcessControlError.fromErrno(errno)) + } + } +} + +// MARK: - Process Control Errors + +enum ProcessControlError: Error, LocalizedError { + case processNotFound + case processChanged + case permissionDenied + case unknownError(Int32) + + var errorDescription: String? { + switch self { + case .processNotFound: + return "Process no longer exists" + case .processChanged: + return "Process has changed (PID was reused by another process)" + case .permissionDenied: + return "Permission denied" + case .unknownError(let code): + return "Operation failed (error code: \(code))" + } + } + + var recoverySuggestion: String? { + switch self { + case .processNotFound: + return "The process may have exited on its own." + case .processChanged: + return "Please refresh the process list and try again." + case .permissionDenied: + return "You don't have permission to control this process. It may be owned by another user or the system." + case .unknownError: + return "Please try again or check system logs." + } + } + + static func fromErrno(_ errno: Int32) -> ProcessControlError { + switch errno { + case ESRCH: + return .processNotFound + case EPERM: + return .permissionDenied + default: + return .unknownError(errno) + } } } diff --git a/TopManager/Views/Processes/ProcessView.swift b/TopManager/Views/Processes/ProcessView.swift index bc8e7c8..47a8d8f 100644 --- a/TopManager/Views/Processes/ProcessView.swift +++ b/TopManager/Views/Processes/ProcessView.swift @@ -26,6 +26,11 @@ struct ProcessView: View { @AppStorage("skipTerminateConfirm") private var skipTerminateConfirm = false @AppStorage("skipForceKillConfirm") private var skipForceKillConfirm = false + // Error handling + @State private var showErrorAlert = false + @State private var errorTitle = "" + @State private var errorMessage = "" + private func updateDisplayedProcesses() { let filtered = monitor.processes.filter { searchText.isEmpty || @@ -144,7 +149,8 @@ struct ProcessView: View { .width(90) } .contextMenu(forSelectionType: ProcessItem.ID.self) { selection in - if let pid = selection.first { + if let pid = selection.first, + let process = monitor.processes.first(where: { $0.pid == pid }) { Button("Terminate (⌫)") { initiateTerminate() } @@ -153,10 +159,10 @@ struct ProcessView: View { } Divider() Button("Suspend") { - monitor.suspendProcess(pid) + performSuspend(process: process) } Button("Resume") { - monitor.resumeProcess(pid) + performResume(process: process) } Divider() Button("Copy PID") { @@ -229,7 +235,7 @@ struct ProcessView: View { isDestructive: true, skipPreferenceKey: "skipTerminateConfirm", onConfirm: { - monitor.terminateProcess(process.pid) + performTerminate(process: process) }, onCancel: {} ) @@ -244,12 +250,17 @@ struct ProcessView: View { isDestructive: true, skipPreferenceKey: "skipForceKillConfirm", onConfirm: { - monitor.forceKillProcess(process.pid) + performForceKill(process: process) }, onCancel: {} ) } } + .alert(errorTitle, isPresented: $showErrorAlert) { + Button("OK", role: .cancel) {} + } message: { + Text(errorMessage) + } } private func initiateTerminate() { @@ -257,7 +268,7 @@ struct ProcessView: View { processToKill = process if skipTerminateConfirm { - monitor.terminateProcess(process.pid) + performTerminate(process: process) } else { showTerminateConfirm = true } @@ -268,12 +279,43 @@ struct ProcessView: View { processToKill = process if skipForceKillConfirm { - monitor.forceKillProcess(process.pid) + performForceKill(process: process) } else { showForceKillConfirm = true } } + private func performTerminate(process: ProcessItem) { + let result = monitor.terminateProcess(process.pid, expectedStartTime: process.startTime) + handleProcessControlResult(result, action: "terminate", processName: process.name) + } + + private func performForceKill(process: ProcessItem) { + let result = monitor.forceKillProcess(process.pid, expectedStartTime: process.startTime) + handleProcessControlResult(result, action: "force kill", processName: process.name) + } + + private func performSuspend(process: ProcessItem) { + let result = monitor.suspendProcess(process.pid, expectedStartTime: process.startTime) + handleProcessControlResult(result, action: "suspend", processName: process.name) + } + + private func performResume(process: ProcessItem) { + let result = monitor.resumeProcess(process.pid, expectedStartTime: process.startTime) + handleProcessControlResult(result, action: "resume", processName: process.name) + } + + private func handleProcessControlResult(_ result: Result, action: String, processName: String) { + switch result { + case .success: + break // Success - no action needed + case .failure(let error): + errorTitle = "Unable to \(action) \"\(processName)\"" + errorMessage = "\(error.errorDescription ?? "Unknown error")\n\n\(error.recoverySuggestion ?? "")" + showErrorAlert = true + } + } + private func cpuColor(_ usage: Double) -> Color { if usage > 80 { return .red