diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 9bd8528..95a0f16 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -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. \ No newline at end of file +**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.## 2024-05-24 - [Fix Path check bypass in SafeStaticFiles] +**Vulnerability:** SafeStaticFiles incorrectly calls `.lower()` on a `Path` object which leads to an `AttributeError` bypassing the security mitigation checks to block access to sensitive directories like `00_CONFIG` and hidden `.env` files. +**Learning:** Python `pathlib.Path` objects do not have string methods like `.lower()`. When building custom path security wrappers, applying string operations to `Path` objects will cause unhandled exceptions that fail open or crash the server. +**Prevention:** Always perform string manipulations like `.lower()` on the string representation of the path before converting it into a `Path` object: `Path(path.lower())` instead of `Path(path).lower()`. Ensure exception handlers for path validations fail securely and catch `AttributeError` and `TypeError`. diff --git a/src/audioformation/server/app.py b/src/audioformation/server/app.py index 9334beb..5cf513b 100644 --- a/src/audioformation/server/app.py +++ b/src/audioformation/server/app.py @@ -24,7 +24,7 @@ class SafeStaticFiles(StaticFiles): async def get_response(self, path: str, scope) -> Response: # Normalize path for check - p = Path(path).lower() + p = Path(path.lower()) if "00_config" in p.parts or p.name.startswith(".env") or ".git" in p.parts: raise HTTPException( status_code=403, detail="Access denied to sensitive resource" diff --git a/src/audioformation/utils/security.py b/src/audioformation/utils/security.py index abcdc30..c15099f 100644 --- a/src/audioformation/utils/security.py +++ b/src/audioformation/utils/security.py @@ -81,7 +81,7 @@ def validate_path_within(path: Path, root: Path) -> bool: return resolved_path.is_relative_to(resolved_root) return False - except (ValueError, RuntimeError, OSError): + except (ValueError, RuntimeError, OSError, TypeError, AttributeError): return False