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 <noreply@anthropic.com>
This commit is contained in:
hariel1985
@@ -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<proc_bsdinfo>.size)
|
||||
let result = proc_pidinfo(pid, PROC_PIDTBSDINFO, 0, &bsdInfo, bsdInfoSize)
|
||||
|
||||
// Check if process still exists
|
||||
guard result == bsdInfoSize else {
|
||||
return .processNotFound
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
func forceKillProcess(_ pid: pid_t) -> Bool {
|
||||
kill(pid, SIGKILL) == 0
|
||||
// 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
|
||||
}
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
func suspendProcess(_ pid: pid_t) -> Bool {
|
||||
kill(pid, SIGSTOP) == 0
|
||||
return nil // Validation passed
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
func resumeProcess(_ pid: pid_t) -> Bool {
|
||||
kill(pid, SIGCONT) == 0
|
||||
func terminateProcess(_ pid: pid_t, expectedStartTime: Date? = nil) -> Result<Void, ProcessControlError> {
|
||||
// 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))
|
||||
}
|
||||
}
|
||||
|
||||
func forceKillProcess(_ pid: pid_t, expectedStartTime: Date? = nil) -> Result<Void, ProcessControlError> {
|
||||
// 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))
|
||||
}
|
||||
}
|
||||
|
||||
func suspendProcess(_ pid: pid_t, expectedStartTime: Date? = nil) -> Result<Void, ProcessControlError> {
|
||||
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<Void, ProcessControlError> {
|
||||
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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Void, ProcessControlError>, 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
|
||||
|
||||
Reference in New Issue
Block a user