T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/data_manager.py:59- Finding
Unrestricted Story Name Allows Directory Creation Outside the Stories Directory
- Content
View full analysis
int: with self.db.get_connection() as conn: cursor = conn.cursor() cursor.execute(''' INSERT INTO stories (name, type, world_type, style, metadata) VALUES (?, ?, ?, ?, ?) ''', (name, story_type, world_type, style, json.dumps(metadata or {}))) conn.commit() story_id = cursor.lastrowid story_dir = STORIES_DIR / name story_dir.mkdir(parents=True, exist_ok=True) return story_id ``` ### Technical Analysis The `name` parameter is accepted from the command line and used directly as a path component. The code does not reject absolute paths, parent-directory components such as `..`, path separators, or symbolic-link-based escapes. Although `STORIES_DIR / name` appears to place the directory beneath the project’s `stories` directory, `pathlib` permits traversal components. An absolute `name` can also replace the preceding base path entirely. Calling `mkdir(parents=True)` then creates the resulting directory hierarchy wherever the running process has write permission. The SQL statement is parameterized and is not vulnerable to SQL injection. The vulnerability is specifically the subsequent filesystem use of the same untrusted value. ### Attack Path 1. An attacker or untrusted automation invokes the data manager’s exposed story creation command. 2. The attacker supplies a traversal or absolute path as the story name, for example: ```bash python scripts/data_manager.py create --type story --name "../../outside-directory" ``` An absolute path could also be supplied: ` ...[truncated 1304 chars]- Remediation
View remediation
Path: if not SAFE_STORY_NAME.fullmatch(name): raise ValueError("Story name contains unsupported characters") root = STORIES_DIR.resolve() candidate = (root / name).resolve() if candidate.parent != root: raise ValueError("Story directory must remain directly under STORIES_DIR") return candidate ``` Then replace the vulnerable operations with: ```python story_dir = safe_story_directory(name) story_dir.mkdir(parents=False, exist_ok=False) ``` Using `exist_ok=False` also prevents silently reusing an existing directory unless that behavior is explicitly required. ]]>