-
Notifications
You must be signed in to change notification settings - Fork 0
π‘οΈ Sentinel: [CRITICAL] Fix Path Traversal validation and DoS in SafeStaticFiles #60
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,7 @@ | ||||||||||
| ## 2025-02-21 - Path Traversal in Mix Endpoint API Parameter | ||||||||||
| **Vulnerability:** The `/projects/{project_id}/mix` API endpoint in `src/audioformation/server/routes.py` accepted a `music` parameter (meant to specify a filename within the `05_MUSIC/generated` directory) but directly passed it to `mix_project` without sanitization. This allowed directory traversal payloads like `../../../etc/passwd` to be used for background music resolution. | ||||||||||
| **Learning:** Even internal API inputs that map strictly to filenames inside an expected directory must be sanitized. A simple check for file existence (`if not bg_music_path.exists():`) is insufficient as it confirms existence but allows looking outside the bounded directory. | ||||||||||
| **Prevention:** Always use established sanitization helpers (like `sanitize_filename`) or bound checks (like `validate_path_within`) for any user-supplied string that forms part of a filesystem path. Ensure bypass parameters like `FORCE_NO_MUSIC` are handled before and mutually exclusively from sanitization. | ||||||||||
| **Prevention:** Always use established sanitization helpers (like `sanitize_filename`) or bound checks (like `validate_path_within`) for any user-supplied string that forms part of a filesystem path. Ensure bypass parameters like `FORCE_NO_MUSIC` are handled before and mutually exclusively from sanitization.## 2025-02-23 - Path Traversal Bypass and File Disclosure via Type Confusion | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new entry for 2025-02-23 is appended directly to the end of the previous entry's 'Prevention' section without a newline. This will cause the Markdown renderer to treat the new header as plain text on the same line. Please add a newline between the entries to ensure correct formatting.
Suggested change
|
||||||||||
| **Vulnerability:** The `validate_path_within` function used string manipulation (`os.path.abspath`) combined with `.lower()` to validate paths. This can be bypassed by complex symlink structures or edge-case path variations. Additionally, `SafeStaticFiles` in `src/audioformation/server/app.py` crashed due to a type error (`AttributeError` calling `.lower()` on a `Path` object), resulting in a Denial of Service and bypassing the file blocklist checks completely. | ||||||||||
| **Learning:** String comparisons should never be used as the primary mechanism for path validation. Catching exceptions broadly when manipulating paths prevents 500 errors and information leakage (e.g. stack traces). Python `pathlib.Path` objects do not possess string methods; type checking and method validity are critical around security functions. | ||||||||||
| **Prevention:** Always use `pathlib.Path.resolve().is_relative_to()` for path boundaries. Always coerce inputs to the expected type (e.g., `Path(str(input).lower())`) prior to performing security validation checks. Broaden exception handling to catch multiple types of failures in security boundaries and fail closed. | ||||||||||
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 new vulnerability entry header (
## 2025-02-23) is appended directly to the end of the previous line. This will prevent it from being rendered as a header in Markdown. Please add a newline before the new entry to maintain proper document structure.