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-43, with vulnerable file operations atscripts/graph_manager.py:52-56,scripts/graph_manager.py:74-80, andscripts/graph_manager.py:280-283
Vulnerability Type: Path traversal allowing files outside the graph storage directory to be read, overwritten, or deleted
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")The generated path is then used by read, write, and deletion operations without validation:
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)python 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 directly concatenated into a filesystem path. There is no allowlist validation, separator rejection, canonical-path comparison, or verification that the resolved file remains underself.storage_dir.A value containing traversal components, such as
../../target, produces a path equivalent to:text data/graphs/../../target.jsonOperating-system path resolution can consequently escape
data/graphs. The vulnerable path is consumed by_load_graph(),_save_graph(), anddelete_graph(), exposing read, write, and deletion primitives.The normal ...[truncated 1869 chars]
- Remediation
View remediation
Remediation Suggestions
-
Apply strict allowlist validation to every externally supplied pipeline identifier. For example, accept only identifiers matching
^pipeline_[A-Za-z0-9_-]+$. -
Explicitly reject absolute paths, path separators, null bytes,
.components, and..components. -
Resolve both the storage root and candidate path canonically, then verify containment before every operation:
python from pathlib import Path import re PIPELINE_ID_PATTERN = re.compile(r"^pipeline_[A-Za-z0-9_-]+$") def _get_graph_path(self, pipeline_id: str) -> Path: if not PIPELINE_ID_PATTERN.fullmatch(pipeline_id): raise ValueError("Invalid pipeline identifier") root = Path(self.storage_dir).resolve() candidate = (root / f"{pipeline_id}.json").resolve() if candidate.parent != root: raise ValueError("Graph path escapes the storage directory") return candidate -
Perform the containment check centrally so reads, writes, and deletions cannot accidentally use different validation rules.
-
Run the process with a dedicated, minimally privileged account whose filesystem access is restricted to application data.
-
Consider removing or separately authorizing
delete_graph()when graph identifiers can originate from untrusted callers. -
Add tests covering relative traversal, absolute paths, mixed separators, symbolic links, encoded separators, and malformed identifiers.
-