🛡️ Sentinel: [HIGH] Fix TOCTOU vulnerability in config permissions#297
🛡️ Sentinel: [HIGH] Fix TOCTOU vulnerability in config permissions#297acebytes wants to merge 1 commit into
Conversation
🚨 Severity: HIGH 💡 Vulnerability: A Time-of-Check to Time-of-Use (TOCTOU) vulnerability where `chmod` was used to enforce permissions, potentially following symlinks to alter external files. 🎯 Impact: An attacker could place a symlink at the config path, causing the daemon to inadvertently change permissions of sensitive system files to 0600. 🔧 Fix: Replaced `chmod` with a secure sequence: opening the file descriptor with `O_NOFOLLOW` and `O_CLOEXEC`, then calling `fchmod` on the descriptor. ✅ Verification: Tests run successfully, verified `open` and `fchmod` usage in `DaemonMode.swift`. Co-authored-by: acebytes <2820910+acebytes@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bed2ac334
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Enforce 0600 permissions safely to avoid TOCTOU symlink vulnerabilities | ||
| URL(fileURLWithPath: path).withUnsafeFileSystemRepresentation { ptr in | ||
| guard let ptr = ptr else { return } | ||
| let fd = open(ptr, O_RDONLY | O_NOFOLLOW | O_CLOEXEC) |
There was a problem hiding this comment.
Opening the config with O_RDONLY before calling fchmod introduces a regression: if the file exists but is owner-write-only (for example mode 0200), open fails with EACCES, so permissions are never corrected to 0600 and the subsequent read path fails. The previous chmod(path, 0o600) flow could repair such files first, then read them. This can break reloads for mis-permissioned but otherwise valid config files.
Useful? React with 👍 / 👎.
| if fd >= 0 { | ||
| fchmod(fd, 0o600) | ||
| close(fd) | ||
| } | ||
| } |
There was a problem hiding this comment.
If open(..., O_NOFOLLOW, ...) fails (for example because path is a symlink), the code silently skips permission enforcement and still proceeds to contents(atPath:), which follows the path and reads data anyway. That means the hardening step can be bypassed entirely in exactly the symlink case this change is trying to secure, and config loading continues without ever enforcing 0600 on the file actually consumed.
Useful? React with 👍 / 👎.
1 participant
🚨 Severity: HIGH
💡 Vulnerability: A Time-of-Check to Time-of-Use (TOCTOU) vulnerability where
chmodwas used to enforce permissions, potentially following symlinks to alter external files.🎯 Impact: An attacker could place a symlink at the config path, causing the daemon to inadvertently change permissions of sensitive system files to 0600.
🔧 Fix: Replaced
chmodwith a secure sequence: opening the file descriptor withO_NOFOLLOWandO_CLOEXEC, then callingfchmodon the descriptor.✅ Verification: Tests run successfully, verified
openandfchmodusage inDaemonMode.swift.PR created automatically by Jules for task 11320394093202304501 started by @acebytes