fix: prevent symlink following in file protocol scanning#7222
fix: prevent symlink following in file protocol scanning#7222sandiyochristan wants to merge 2 commits intoprojectdiscovery:devfrom
Conversation
Use os.Lstat instead of os.Stat and check for symlink mode bits to prevent the file protocol from following symlinks outside the intended scan scope. This prevents information disclosure when scanning directories containing attacker-controlled symlinks.
Neo - PR Security ReviewNo security issues found Highlights
Hardening Notes
Comment |
WalkthroughThis change adds symlink guards in file discovery: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/protocols/file/find.go (1)
50-65:⚠️ Potential issue | 🟠 MajorMissing symlink check in glob matching creates inconsistent security posture.
The
findGlobPathMatchesfunction lacks symlink protection, unlikefindFileMatchesandfindDirectoryMatches. An attacker could create a symlink matching a glob pattern (e.g.,*.txt→/etc/shadow), bypassing the security fix.Consider adding an
os.Lstatcheck before processing each match:🛡️ Proposed fix to add symlink protection
func (request *Request) findGlobPathMatches(absPath string, processed map[string]struct{}, callback func(string)) error { matches, err := filepath.Glob(absPath) if err != nil { return errors.Errorf("wildcard found, but unable to glob: %s\n", err) } for _, match := range matches { + // Skip symlinks to prevent traversal outside scan scope + info, err := os.Lstat(match) + if err != nil { + continue + } + if info.Mode()&os.ModeSymlink != 0 { + continue + } if !request.validatePath(absPath, match, false) { continue } if _, ok := processed[match]; !ok { processed[match] = struct{}{} callback(match) } } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/file/find.go` around lines 50 - 65, findGlobPathMatches is missing the symlink protection present in findFileMatches/findDirectoryMatches: before accepting each match returned by filepath.Glob, perform an os.Lstat on the match and skip it if the FileMode indicates a symlink (mode&os.ModeSymlink != 0); only after passing the lstat check should you call request.validatePath, update the processed map, and invoke the callback(match) so symlinked targets are not followed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@pkg/protocols/file/find.go`:
- Around line 50-65: findGlobPathMatches is missing the symlink protection
present in findFileMatches/findDirectoryMatches: before accepting each match
returned by filepath.Glob, perform an os.Lstat on the match and skip it if the
FileMode indicates a symlink (mode&os.ModeSymlink != 0); only after passing the
lstat check should you call request.validatePath, update the processed map, and
invoke the callback(match) so symlinked targets are not followed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7cadbf8-6a78-4313-b3d0-c75d4ed3a5f4
📒 Files selected for processing (1)
pkg/protocols/file/find.go
findGlobPathMatches was missing the symlink guard present in findFileMatches and findDirectoryMatches. A symlink matching a glob pattern (e.g., *.txt -> /etc/shadow) would bypass the fix.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/protocols/file/find.go (1)
56-63: Symlink guard implementation looks correct.The use of
os.Lstatand the bitwise checkinfo.Mode()&os.ModeSymlink != 0correctly identifies symlinks without following them.One minor observation: unlike the denylist skip at line 157 which logs at verbose level, skipped symlinks are silent. Consider adding a
gologger.Verbose()message for easier debugging in complex scan scenarios.♻️ Optional: Add verbose logging for skipped symlinks
if info.Mode()&os.ModeSymlink != 0 { + gologger.Verbose().Msgf("Skipping symlink: %s\n", match) continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/file/find.go` around lines 56 - 63, Add a verbose log when a file is skipped due to being a symlink: inside the symlink guard that uses os.Lstat and checks info.Mode()&os.ModeSymlink, call gologger.Verbose() (mirroring the denylist skip behavior) to emit a short message indicating the path was skipped as a symlink for easier debugging during scans.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/protocols/file/find.go`:
- Around line 56-63: Add a verbose log when a file is skipped due to being a
symlink: inside the symlink guard that uses os.Lstat and checks
info.Mode()&os.ModeSymlink, call gologger.Verbose() (mirroring the denylist skip
behavior) to emit a short message indicating the path was skipped as a symlink
for easier debugging during scans.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e8f3a5a-ad90-4225-b2cb-328e40d3c91e
📒 Files selected for processing (1)
pkg/protocols/file/find.go
dwisiswant0
left a comment
dwisiswant0
left a comment
There was a problem hiding this comment.
Please file an issue before submitting fix patches.
Include reproduction steps as per the template and the exact nuclei command (or custom runner PoC for Nuclei SDK) that triggers unexpected behavior. The reproduction must be based on a realistic execution path that is fully concrete and reproducible.
Thanks!
2 participants
Proposed Changes
The file protocol scanner follows symlinks when enumerating files, allowing information disclosure of files outside the intended scan scope (CWE-59: Improper Link Resolution Before File Access).
Root Cause
In
pkg/protocols/file/find.go, thefindFileMatchesfunction usesos.Stat()(line 70) which transparently follows symlinks. When a user scans a directory like/var/www/, an attacker who can write to that directory can place symlinks pointing to sensitive files:When nuclei scans
/var/www/with the file protocol,os.Stat()follows these symlinks and nuclei processes the target files as if they were inside the scan directory.Attack Scenarios
Scenario 1: Shared hosting environment
.env,credentials.json)Scenario 2: CI/CD pipeline scanning
The Fix
findFileMatches— Changedos.Stat()toos.Lstat()which does NOT follow symlinks, and added an explicit check foros.ModeSymlinkto skip symlinked files:findDirectoryMatches— Addedd.Type()&os.ModeSymlinkcheck in thefilepath.WalkDircallback to skip symlinked entries during recursive directory traversal:Note:
filepath.WalkDiralready usesos.Lstatinternally (per Go docs), sod.Type()correctly reports symlinks. The explicit check ensures they are skipped rather than processed.Files Changed
pkg/protocols/file/find.go—os.Stat→os.LstatinfindFileMatches, symlink skip checks in bothfindFileMatchesandfindDirectoryMatchesSecurity Impact
Proof
os.Lstatis the standard Go approach for symlink-safe file operations — it returns symlink info without resolving the targetos.ModeSymlinkbit check is the idiomatic Go pattern for detecting symlinksfilepath.WalkDiralready provides symlink information viad.Type(), so the check has zero performance overheadChecklist
devbranchSummary by CodeRabbit