T09 · Insecure Skill Coding Practices
- Location
scripts/kb_manager.py:246- Finding
User-Controlled Category Enables Filesystem Writes Outside the Knowledge-Base Directory
- Content
View full analysis
bool: """Add a new category.""" ensure_kb_exists() index = load_index() if name not in index["categories"]: index["categories"].append(name) (KB_ROOT / name).mkdir(exist_ok=True) save_index(index) return True return False ``` The affected values are reachable through user-facing entry points, including: - `scripts/kb_cli.py`, through `ingest --category` and `add-category` - `scripts/batch_ingest.py`, through `--category` - `scripts/lark_bridge.py`, through the `category` argument ### Technical Analysis `category` and category `name` values are used directly as filesystem path components without validation or containment checks. Python's `pathlib` does not guarantee that `KB_ROOT / category` remains under `KB_ROOT`. If `category` is an absolute path, the absolute operand replaces the preceding root. Relative v ...[truncated 1882 chars]- Remediation
View remediation
Path: if not category or Path(category).is_absolute(): raise ValueError("Invalid category") if category in {".", ".."} or "/" in category or "\\" in category: raise ValueError("Category must be a single directory name") root = KB_ROOT.resolve() destination = (root / category).resolve() if not destination.is_relative_to(root): raise ValueError("Category escapes the knowledge-base directory") return destination ``` 5. Apply the same validation helper in both `ingest_file()` and `add_category()`. 6. Before writing the Markdown file, resolve `md_path` and perform a second containment check against the resolved knowledge-base root. 7. Consider refusing symlinked category directories, or verify that their resolved targets remain under `KB_ROOT`. 8. Add regression tests covering absolute paths, `../` traversal, nested paths, path separators, and symlink escapes. ]]>
