fix: prevent path traversal via task_id in cleaning task log/delete e…#508
Merged
Conversation
…ndpoints Add UUID format validation in check_task_id() and resolve+startsWith boundary checks on all log_path/task_path constructions to prevent arbitrary file read and arbitrary directory deletion. - cleaning_task_validator.py: add UUID regex to check_task_id() - cleaning_task_service.py: add path boundary checks in get_task_log() and delete_task() - cleaning_task_routes.py: add path boundary checks in stream and download endpoints
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ile write) Add normalize + startsWith boundary checks in ChunksSaver save/saveFile methods to prevent directory traversal via crafted fileName containing '../'. - ChunksSaver.java: validate final path stays within uploadPath in save() and saveFile() - chunks_saver.py: validate resolved path stays within upload_path in save() and save_file()
…d destPath injection Add multi-layer path boundary checks to prevent cross-directory reads and writes: - GlusterfsWriter: validate destPath stays within /dataset, reject .. in subPath, validate sourcePath stays within mount point, reject path separators in fileName - GlusterfsReader: validate readPath stays within mount point
Add .. rejection in ValidPathValidator and ValidFilePathValidator, plus normalize+startsWith boundary check on uploadPath in preUpload to prevent cross-directory writes via crafted prefix parameters. - ValidPathValidator: reject .. sequences - ValidFilePathValidator: reject .. sequences - DatasetFileApplicationService.preUpload(): validate uploadPath stays within datasetBasePath
Restructure path construction to validate task_id BEFORE building Path objects, use Path / operator instead of f-string concatenation, and use Path.relative_to() for boundary checks instead of str.startswith(). - Routes: add inline re.fullmatch validation before Path construction - Service: use flow_root / task_id / filename pattern - All 4 locations: replace startswith() with relative_to()
Replace inline re.fullmatch with module-level _TASK_ID_PATTERN and replace relative_to() try/except with parents containment check to satisfy CodeQL taint tracking as proper sanitizers. - routes: module-level _TASK_ID_PATTERN + flow_base not in log_path.parents - service: flow_root not in parents for get_task_log and delete_task
…uction Replace inline regex validation with _sanitize_task_id() / sanitize_task_id() functions whose return values CodeQL tracks as sanitized, eliminating the remaining 6 "Uncontrolled data used in path expression" warnings.
Add _sanitize_retry_count() / sanitize_retry_count() with range validation and use the sanitized return value in all 5 places where retry_count feeds into log_path construction. CodeQL tracks the sanitizer return value as safe.
CodeQL cannot trace data flow through instance-method calls (self.validator. sanitize_*). Replace with module-level _sanitize_task_id / _sanitize_retry_count so CodeQL recognizes the sanitized return values.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ndpoints
Add UUID format validation in check_task_id() and resolve+startsWith boundary checks on all log_path/task_path constructions to prevent arbitrary file read and arbitrary directory deletion.
close: #507