🛡️ Sentinel: [HIGH] Fix TOCTOU vulnerability in config permission enforcement#287
🛡️ Sentinel: [HIGH] Fix TOCTOU vulnerability in config permission enforcement#287acebytes wants to merge 1 commit into
Conversation
Replaced path-based `chmod()` with a file-descriptor-based approach using `open()` with `O_NOFOLLOW` followed by `fchmod()` to ensure permissions are only applied to the actual file and never follow symlinks. 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: d34d586ec1
ℹ️ 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 { cPath in | ||
| guard let cPath = cPath else { return } | ||
| let fd = open(cPath, O_RDONLY | O_NOFOLLOW) |
There was a problem hiding this comment.
Opening autopilot.json with open(..., O_RDONLY | O_NOFOLLOW) can block indefinitely if an attacker replaces the path with a FIFO (POSIX open on FIFO read end blocks until a writer appears). Because this runs in the config reload/startup path (loadConfig), the daemon can hang before reporting status, which is a new denial-of-service vector introduced by replacing non-blocking chmod(path, ...) with a blocking open call.
Useful? React with 👍 / 👎.
| // Enforce 0600 permissions safely to avoid TOCTOU symlink vulnerabilities | ||
| URL(fileURLWithPath: path).withUnsafeFileSystemRepresentation { cPath in | ||
| guard let cPath = cPath else { return } | ||
| let fd = open(cPath, O_RDONLY | O_NOFOLLOW) |
There was a problem hiding this comment.
Using open(..., O_RDONLY | O_NOFOLLOW) as the prerequisite for fchmod means files that are owner-owned but currently unreadable (for example mode 000 or 0200) cannot be repaired to 0600 by this code path. The old implementation changed mode first and then read, but the new path logs a warning and then fails contents(atPath:), turning a recoverable mispermission into a persistent config load failure until manual intervention.
Useful? React with 👍 / 👎.
1 participant
🚨 Severity: HIGH
💡 Vulnerability: Time-of-Check to Time-of-Use (TOCTOU) symlink vulnerability when enforcing 0600 permissions on the autopilot configuration file.
🎯 Impact: A malicious local user could replace the configuration file with a symlink pointing to an arbitrary system file, causing the daemon to inadvertently change the target file's permissions to 0600, potentially leading to denial of service or privilege escalation.
🔧 Fix: Replaced path-based
chmod()with a file-descriptor-based approach usingopen()withO_NOFOLLOWfollowed byfchmod()to ensure permissions are only applied to the actual file and never follow symlinks.✅ Verification: Verified the code compiles and all existing tests pass using
./scripts/test.sh.PR created automatically by Jules for task 12485612190212364064 started by @acebytes