🛡️ Sentinel: [HIGH] Fix TOCTOU vulnerability in file permissions#290
🛡️ Sentinel: [HIGH] Fix TOCTOU vulnerability in file permissions#290acebytes wants to merge 1 commit into
Conversation
Used `open()` with `O_NOFOLLOW` flag to safely obtain a file descriptor and then applied permissions using `fchmod()` instead of using `chmod()` directly on a file path, preventing Time-of-Check to Time-of-Use (TOCTOU) symlink attacks. 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: f16a001af6
ℹ️ 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".
| let url = URL(fileURLWithPath: path) | ||
| url.withUnsafeFileSystemRepresentation { cPath in | ||
| guard let cPath = cPath else { return } | ||
| let fd = open(cPath, O_RDONLY | O_NOFOLLOW) |
There was a problem hiding this comment.
O_RDONLY when opening file for fchmod
Opening the config file with O_RDONLY | O_NOFOLLOW can fail with EACCES when the file is owner-owned but currently not readable (for example mode 0000/0200), so the permission fix is skipped and the subsequent contents(atPath:) read fails. This is a regression from the previous chmod(path, 0o600) behavior, which could recover such files by tightening permissions first, and it can break startup/reload for mis-permissioned autopilot configs.
Useful? React with 👍 / 👎.
| url.withUnsafeFileSystemRepresentation { cPath in | ||
| guard let cPath = cPath else { return } | ||
| let fd = open(cPath, O_RDONLY | O_NOFOLLOW) | ||
| if fd >= 0 { |
There was a problem hiding this comment.
O_NOFOLLOW open fails
The new secure-open block silently ignores open(..., O_NOFOLLOW) failures and proceeds to load the config by path, so a symlinked autopilot.json now bypasses the intended 0600-hardening step entirely while still being consumed. Because O_NOFOLLOW is specifically meant to fail on symlinks, this needs an explicit error path instead of continuing with contents(atPath:) after fd < 0.
Useful? React with 👍 / 👎.
1 participant
🚨 Severity: HIGH
💡 Vulnerability: Used
chmod()on a file path directly, which is vulnerable to Time-of-Check to Time-of-Use (TOCTOU) attacks. An attacker could replace the target file with a symlink to another file just beforechmodexecutes.🎯 Impact: An attacker could exploit the race condition to change the permissions of an arbitrary file they shouldn't have access to.
🔧 Fix: Replaced
chmod()withopen()using theO_NOFOLLOWflag to obtain a file descriptor safely, then applied permissions usingfchmod().✅ Verification: Run
./scripts/test.shto ensure no functionality is broken and review the code to ensurechmod()is no longer used for this file path.PR created automatically by Jules for task 1910692162396585709 started by @acebytes