🛡️ Sentinel: [HIGH] Fix Arbitrary File Read in Archive Module#777
🛡️ Sentinel: [HIGH] Fix Arbitrary File Read in Archive Module#777
Conversation
The archive module previously followed symbolic links when creating archives, which could lead to sensitive file disclosure if a symlink to a sensitive file was present in the source directory. This change modifies `ArchiveModule` to check for symbolic links using `fs::symlink_metadata` and store them as symbolic links in the archive (for both tar and zip formats), rather than following them and storing the target content. A regression test `test_archive_symlink_security` has been added to verify the fix. Co-authored-by: dolagoartur <146357947+dolagoartur@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: d9f2a96bf8
ℹ️ 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".
| header.set_path(relative_path)?; | ||
| header.set_link_name(target)?; |
There was a problem hiding this comment.
The symlink branch now constructs tar headers directly with set_path/set_link_name, which skips the builder’s GNU long-name/long-link handling and causes archive creation to fail when a symlink path or target exceeds tar header limits (a realistic case in deeply nested dependency trees). In this function, long regular-file paths still work because append_path_with_name emits the extension records, so this is a regression specific to symlink entries.
Useful? React with 👍 / 👎.
1 participant
🛡️ Sentinel: [CRITICAL] Fix Arbitrary File Read via Symlink in Archive Module
🚨 Severity: HIGH
💡 Vulnerability: The
archivemodule followed symbolic links when creating archives. If an attacker could place a symlink in the source directory pointing to a sensitive file (e.g.,/etc/passwd), the archive would contain the content of that file.🎯 Impact: Arbitrary file read / Information disclosure.
🔧 Fix: Modified
ArchiveModuleto usefs::symlink_metadatainstead offs::metadatato detect symlinks.tararchives: Added logic to create atar::HeaderwithEntryType::Symlinkand set the link name, appending it without content.ziparchives: Added logic to set Unix permissions0o120777(S_IFLNK) and write the target path as the file content, which is the standard way to store symlinks in zip files.✅ Verification: Added a regression test
test_archive_symlink_securityinsrc/modules/archive.rsthat creates a symlink to a sensitive file and asserts that the resulting archive contains a symlink entry and DOES NOT contain the sensitive content. Confirmed that the test fails before the fix and passes after the fix.PR created automatically by Jules for task 7524086818539629066 started by @dolagoartur