From 9d9011e6d43bdb163ec5b282a4494c79a0347adb Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 13 Jun 2026 06:17:25 +0000 Subject: [PATCH] Fixed TOCTOU vulnerability in file creation. Co-authored-by: acebytes <2820910+acebytes@users.noreply.github.com> --- .jules/sentinel.md | 4 +++ Sources/CacheoutHelperLib/SysctlJournal.swift | 26 ++++++++++++++----- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index fd58571..12c68d0 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -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-06-13 - TOCTOU Vulnerability in File Creation +**Vulnerability:** Found insecure file creation using `Data.write(to:)` followed by `FileManager.default.setAttributes()`, creating a Time-Of-Check to Time-Of-Use window where the file is created with global umask permissions before being restricted. +**Learning:** `Data.write(to:)` creates files using the process's default umask. For privileged daemons, this exposes temporary files to unprivileged users before permissions are tightened. +**Prevention:** Use POSIX `open()` with `O_CREAT | O_WRONLY | O_EXCL | O_CLOEXEC` flags and explicitly set the target mode (e.g., `0600`), then wrap the returned file descriptor in a `FileHandle`. diff --git a/Sources/CacheoutHelperLib/SysctlJournal.swift b/Sources/CacheoutHelperLib/SysctlJournal.swift index a14c902..80f5a68 100644 --- a/Sources/CacheoutHelperLib/SysctlJournal.swift +++ b/Sources/CacheoutHelperLib/SysctlJournal.swift @@ -307,14 +307,26 @@ 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) + // Remove any stale temp file to prevent O_EXCL failure + 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 - ) + // Write to temp file securely (0600) to prevent TOCTOU access + let fd = tmpURL.withUnsafeFileSystemRepresentation { pathPtr -> Int32 in + guard let pathPtr = pathPtr else { return -1 } + return open(pathPtr, O_CREAT | O_WRONLY | O_EXCL | O_CLOEXEC, 0o600) + } + guard fd >= 0 else { + return false + } + + let handle = FileHandle(fileDescriptor: fd, closeOnDealloc: true) + if #available(macOS 10.15.4, *) { + try handle.write(contentsOf: data) + try handle.close() + } else { + handle.write(data) + handle.closeFile() + } // Atomic rename(2) — atomicity on APFS/HFS+. if rename(tmpURL.path, url.path) != 0 {