fix: path traversal vulnerability in is_dangerous_path()
Summary
- Fixes path traversal vulnerability (CWE-22) where subdirectories of dangerous paths were not blocked
is_dangerous_path()now uses prefix matching instead of exact string matching- For example,
/etc/passwdis now correctly blocked when/etcis in DANGEROUS_PATHS
Fixes
- Fixes #312
- Fixes #293
Changes
- Modified
utils/security_config.py: Changedis_dangerous_path()to use prefix matching - Added
tests/test_path_traversal_security.py: 8 test cases covering the vulnerability fix
Test plan
- [x] All existing tests pass (18 file protection tests)
- [x] All new security tests pass (8 tests)
- [x] Verified
/etc/passwd,/etc/shadow,/var/log/auth.logare now blocked - [x] Verified safe paths like
/tmp/testare still allowed - [x] Verified similar names like
/tmp/etcbackupare not incorrectly blocked
Summary of Changes
Hello @DragonFSKY, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the application's security by fixing a critical path traversal vulnerability. The is_dangerous_path() utility function has been refactored to perform robust prefix matching, ensuring that not only explicitly listed dangerous directories but also all their subdirectories are correctly identified and blocked. This prevents unauthorized access to sensitive system files and improves the overall security posture against common attack vectors.
Highlights
- Security Fix: Addressed a path traversal vulnerability (CWE-22) in the
is_dangerous_path()function. - Improved Path Validation: The
is_dangerous_path()function now uses prefix matching instead of exact string matching, ensuring that subdirectories of dangerous paths are correctly blocked (e.g.,/etc/passwdis blocked if/etcis a dangerous path). - New Test Coverage: Introduced a new test file,
tests/test_path_traversal_security.py, with 8 new test cases to validate the fix and prevent regressions.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
[^1]: Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.
@P0LESTAR Thank you for your detailed review and catching this important issue!
You're right - the Windows path separator handling has a flaw. When C:\ is in DANGEROUS_PATHS, appending \ creates C:\\ (double backslash), which doesn't match paths like C:\Users.
I'll fix this by using Path.is_relative_to() (available in Python 3.9+, which is our minimum requirement) instead of string prefix matching. This provides correct cross-platform path comparison.
Could you please help verify the fix once I push the update? 🙏
@gemini-code-assist @chatgpt-codex-connector Regarding the /home concern - I've verified that this is a false positive. The code flow in resolve_and_validate_path() is:
- First checks
is_dangerous_path()- blocks/homeitself - Then checks
is_home_directory_root()- blocks/home/username - Subdirectories like
/home/username/projectpass both checks and are allowed
The existing tests in TestHomeDirectoryProtection confirm this behavior works correctly.
Hello @DragonFSKY,
Thank you for the detailed update and clarification!
Regarding the Windows path separator issue, using Path.is_relative_to() is an excellent approach to ensure robust and cross-platform path comparison. This will definitely improve the reliability of the is_dangerous_path() function. Please do tag me again once you've pushed the update, and I'll be happy to review the changes.
I also appreciate the clarification on the /home concern. Your explanation of the resolve_and_validate_path() flow, specifically how is_dangerous_path() blocks /home itself and is_home_directory_root() handles /home/username, makes perfect sense. It's good to know that the existing tests confirm this correct behavior.
Looking forward to seeing the updated changes!
LGTM. It works fine on both Linux and Windows.
Where is "EXCLUDED_DIRS" used?
is pre-existing code, not added by this PR. It's used to skip directories like node_modules, .git, __pycache__ during recursive file search.
EXCLUDED_DIRS is pre-existing code, not added by this PR. It is used to skip directories like node_modules, .git, pycache during recursive file search.
Oh, that's right. I was mistaken
@codex review
@codex review
Please re-analyze this PR after the latest fix commit (e5548ac).
The fix addresses your previous feedback about blocking all home directory subpaths by:
- Splitting DANGEROUS_PATHS into DANGEROUS_SYSTEM_PATHS and DANGEROUS_HOME_CONTAINERS
- System paths (e.g., /etc) block the path AND all subdirectories
- Home containers (e.g., /home) block ONLY the exact path, allowing /home/user/project to pass through
This ensures users can still access their project directories under /home/* while maintaining security.
@guidedways Thanks for the reminder! Fixed in e5548ac.
Codex Review: Didn't find any major issues. Nice work!
ℹ️ 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".
@DragonFSKY
adding "C:\" to the “block subdirs” set effectively blocks almost all real-world absolute Windows workspaces, same issue on windows
Pushed follow-up fix commit ba08308 on top of this PR (maintainer push).\n\n- Fixes macOS symlinked system dirs (/etc -> /private/etc, /var -> /private/var) so prefix-blocking works as intended\n- Avoids overblocking Windows workspaces by not treating C:\ as a prefix-blocked system path (drive root still blocked via root check)\n- Adjusts macOS test TMPDIR to /tmp so tests don't create workspaces under /private/var when /var is blocked\n\nRan: pytest tests/test_path_traversal_security.py tests/test_file_protection.py