-
Notifications
You must be signed in to change notification settings - Fork 277
Potential fix for code scanning alert no. 16: Uncontrolled data used in path expression #439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -211,11 +211,13 @@ def _serve_static_file(self, path): | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Fallback to file system if not cached | ||||||||||||||||||||||||||||||||
| portal_dir = os.path.dirname(os.path.abspath(__file__)) | ||||||||||||||||||||||||||||||||
| static_dir = os.path.join(portal_dir, 'static') | ||||||||||||||||||||||||||||||||
| full_path = os.path.join(static_dir, file_path) | ||||||||||||||||||||||||||||||||
| # Normalize static directory path | ||||||||||||||||||||||||||||||||
| static_dir = os.path.realpath(os.path.join(portal_dir, 'static')) | ||||||||||||||||||||||||||||||||
| # Build and normalize full path to requested file | ||||||||||||||||||||||||||||||||
| full_path = os.path.realpath(os.path.join(static_dir, file_path)) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Security check: ensure file is within static directory | ||||||||||||||||||||||||||||||||
| if not os.path.abspath(full_path).startswith(static_dir): | ||||||||||||||||||||||||||||||||
| if os.path.commonpath([static_dir, full_path]) != static_dir: | ||||||||||||||||||||||||||||||||
| self._send_error_response(403, 'Forbidden') | ||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
Comment on lines
+220
to
223
|
||||||||||||||||||||||||||||||||
| if os.path.commonpath([static_dir, full_path]) != static_dir: | |
| self._send_error_response(403, 'Forbidden') | |
| return | |
| try: | |
| common_path = os.path.commonpath([static_dir, full_path]) | |
| except ValueError: | |
| # Paths may be on different drives or otherwise incompatible; deny access. | |
| self._send_error_response(403, 'Forbidden') | |
| return | |
| if common_path != static_dir: | |
| self._send_error_response(403, 'Forbidden') | |
| return | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The security fix for path traversal lacks test coverage. Consider adding tests to verify that path traversal attempts (e.g., requests to '/static/../../../etc/passwd', '/static/..\..\..\windows\system32\config\sam') are properly blocked and return 403 Forbidden. This would help prevent regressions and validate the fix works correctly across different operating systems.