Skip to content
Open
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
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,7 @@
**Vulnerability:** External shell command executed in `listLocalSnapshots()` triggered a deadlock when `tmutil` output exceeded 64KB, because stdout and stderr were read synchronously inside the process termination handler.
**Learning:** In Swift, reading from a process pipe synchronously inside a `terminationHandler` can result in a permanent deadlock if the child blocks writing to a full pipe, preventing it from exiting.
**Prevention:** Asynchronously drain pipes continuously while the process is running using background queues.
## 2026-05-21 - TOCTOU Vulnerability via Data.write and setAttributes
**Vulnerability:** Found `try data.write(to: tmpURL)` followed by `try FileManager.default.setAttributes([.posixPermissions: 0o600], ...)` in `flushState()`, which introduces a TOCTOU window where the file is exposed with default umask permissions before being tightened.
**Learning:** `Data.write(to:)` is vulnerable to TOCTOU if permissions need to be restricted after creation.
**Prevention:** Use POSIX `open()` with `O_CREAT | O_WRONLY | O_EXCL | O_CLOEXEC` and the `0o600` mode to securely create files without relying on the process umask, then wrap the file descriptor in a `FileHandle`.
24 changes: 17 additions & 7 deletions Sources/CacheoutHelperLib/SysctlJournal.swift
Original file line number Diff line number Diff line change
Expand Up @@ -307,14 +307,24 @@ public final class SysctlJournal {
do {
let data = try PropertyListEncoder().encode(state)

// Write to temp file (non-atomic β€” we control the rename ourselves).
try data.write(to: tmpURL)
try? FileManager.default.removeItem(at: tmpURL)

// Set permissions to 0600 (root-only) on temp file before rename.
try FileManager.default.setAttributes(
[.posixPermissions: 0o600],
ofItemAtPath: tmpURL.path
)
let fd = tmpURL.withUnsafeFileSystemRepresentation { pathPtr -> Int32 in
guard let ptr = pathPtr else { return -1 }
return open(ptr, O_CREAT | O_WRONLY | O_EXCL | O_CLOEXEC, S_IRUSR | S_IWUSR)
}
guard fd >= 0 else {
logger.error("Failed to securely open temp journal file: errno \(errno, privacy: .public)")
return false
}

let handle = FileHandle(fileDescriptor: fd, closeOnDealloc: true)
handle.write(data)
Comment on lines +321 to +322

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use the throwing FileHandle write API

When the temp journal write itself fails, for example because /var/run is out of space or returns an I/O error, this non-throwing FileHandle.write(_:) path does not report a Swift error to the surrounding catch the way the previous Data.write(to:) did. That can leave flushState() unable to return false cleanly for record(_:), either aborting the helper or missing the durable-write failure before the sysctl change. Please use the throwing write(contentsOf:) API and handle the error like the open/rename failures.

Useful? React with πŸ‘Β / πŸ‘Ž.

if #available(macOS 10.15.4, *) {
try? handle.close()
Comment on lines +323 to +324

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not ignore close failures before committing journal

On macOS 14 this branch is always used, and try? handle.close() discards delayed write/close errors such as ENOSPC, quota, or I/O failures while flushing the file. flushState() then proceeds to rename the temp file and return true, so record(_:) can allow the sysctl write even though the journal state was not durably written, defeating the rollback guarantee. Please treat a thrown close as a failed flush and remove the temp file instead of committing it.

Useful? React with πŸ‘Β / πŸ‘Ž.

} else {
handle.closeFile()
}

// Atomic rename(2) β€” atomicity on APFS/HFS+.
if rename(tmpURL.path, url.path) != 0 {
Expand Down
Loading