T09 · Insecure Skill Coding Practices
- Location
scripts/graph_manager.py:41- Finding
Path Traversal in Graph File Operations
- Content
View full analysis
Vulnerability Details
File Location:
scripts/graph_manager.py:41-42,scripts/graph_manager.py:52-56,scripts/graph_manager.py:74-80,scripts/graph_manager.py:280-284
Vulnerability Type: Path traversal leading to unauthorized file read, write, and deletion
Risk Level: HighVulnerable Code
python def _get_graph_path(self, pipeline_id: str) -> str: """Get the graph file path.""" return os.path.join(self.storage_dir, f"{pipeline_id}.json")python graph_path = self._get_graph_path(pipeline_id) if os.path.exists(graph_path): try: with open(graph_path, 'r', encoding='utf-8') as f: data = json.load(f) self.cache[pipeline_id] = data return datapython def _save_graph(self, pipeline_id: str, data: Dict): """Save graph data.""" graph_path = self._get_graph_path(pipeline_id) self.cache[pipeline_id] = data with open(graph_path, 'w', encoding='utf-8') as f: json.dump(data, f, ensure_ascii=False, indent=2)python graph_path = self._get_graph_path(pipeline_id) if os.path.exists(graph_path): os.remove(graph_path) return {"success": True, "message": "Graph deleted"}Technical Analysis
pipeline_idis incorporated directly into a filesystem path without validating its format, rejecting path separators, or verifying that the resulting canonical path remains understorage_dir.A value containing traversal components such as
../can escape the intendeddata/graphsdirectory. An absolute path can also causeos.path.join()to discard the storage-directory prefix. The implementation then uses the resulting path in read, write, and deletion operations.The
.jsonsuffix limits the vulnerable operations to paths ending in.json, and reads require valid JSON because the code callsjson.load(). These restrictions do not prevent access to other JSON co ...[truncated 1216 chars]- Remediation
View remediation
Remediation Suggestions
- Enforce a strict allowlist for identifiers, for example:
python import re PIPELINE_ID_PATTERN = re.compile(r"\Apipeline_[0-9]{14}\Z") def _validate_pipeline_id(self, pipeline_id: str) -> None: if not PIPELINE_ID_PATTERN.fullmatch(pipeline_id): raise ValueError("Invalid pipeline ID") - Explicitly reject absolute paths, path separators,
.components, and..components. - Resolve and verify canonical paths before every operation:
python def _get_graph_path(self, pipeline_id: str) -> str: self._validate_pipeline_id(pipeline_id) base = os.path.realpath(self.storage_dir) target = os.path.realpath(os.path.join(base, f"{pipeline_id}.json")) if os.path.commonpath([base, target]) != base: raise ValueError("Graph path escapes storage directory") return target - Run the application with a dedicated, least-privileged account that cannot modify unrelated files.
- Use atomic writes through a safely created temporary file in the validated graph directory, followed by
os.replace(). - Add tests covering
../, absolute paths, nested traversal, alternate separators, empty identifiers, and symbolic-link edge cases.
- Enforce a strict allowlist for identifiers, for example: