T09 · Insecure Skill Coding Practices
Error
- Location
scripts/graph_manager.py:33- Finding
Path Traversal Through Unsanitized Pipeline Identifiers
- Content
View full analysis
str: """获取图谱文件路径""" return os.path.join(self.storage_dir, f"{pipeline_id}.json") ``` The generated path is subsequently used without a containment check: ```python def _save_graph(self, pipeline_id: str, data: Dict): """保存图谱数据""" 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) ``` The same unsafe path construction reaches a deletion sink: ```python def delete_graph(self, pipeline_id: str) -> Dict: """删除图谱""" # 清除缓存 if pipeline_id in self.cache: del self.cache[pipeline_id] # 删除文件 graph_path = self._get_graph_path(pipeline_id) if os.path.exists(graph_path): os.remove(graph_path) return {"success": True, "message": "图谱已删除"} return {"success": False, "error": "图谱不存在"} ``` ### Technical Analysis `pipeline_id` is concatenated into a filesystem path without validation, normalization, or verification that the resulting path remains under `data/graphs`. An identifier containing traversal components such as `../` can cause `os.path.join()` to resolve to a file outside the intended graph directory. The implementation automatically appends `.json`, but this still permits access to arbitrary JSON-named files reachable with the process's filesystem permissions. The vulnerable path is used by graph loading, saving, and deletion operations. Consequently, the issue is not limited to disclosure: it can also result in destructive modification or removal ...[truncated 1185 chars]- Remediation
View remediation
None: if not isinstance(pipeline_id, str) or not PIPELINE_ID_PATTERN.fullmatch(pipeline_id): raise ValueError("Invalid pipeline identifier") ``` 2. Canonicalize the base and destination paths and enforce directory containment: ```python def _get_graph_path(self, pipeline_id: str) -> str: _validate_pipeline_id(pipeline_id) base = os.path.realpath(self.storage_dir) destination = os.path.realpath( os.path.join(base, f"{pipeline_id}.json") ) if os.path.commonpath([base, destination]) != base: raise ValueError("Graph path escapes the storage directory") return destination ``` 3. Reject path separators, `.` components, `..` components, absolute paths, null bytes, and platform-specific alternate separators. 4. Apply validation in every public method rather than relying solely on the caller. 5. Use atomic writes through a securely created temporary file in the same directory, followed by `os.replace()`, to reduce corruption risk. 6. Run the Skill under a dedicated low-privilege account with filesystem access limited to its own data directory. 7. Add tests covering Unix and Windows traversal forms, including `../`, `..\`, absolute paths, repeated separators, and encoded or mixed-separator variants. ]]>