T09 · Insecure Skill Coding Practices
Error
- Location
- lib/crawler.py:66
- Finding
- Arbitrary File Write Through Unsafe URL-to-Path Conversion## Vulnerability Details **File Location**: `lib/crawler.py`, lines 66-83 **Vulnerability Type**: Path traversal resulting in arbitrary file write **Risk Level**: High ### Vulnerable Code ```python def url_to_filepath(base_dir: str, url: str) -> str: parsed = urllib.parse.urlparse(url) path = parsed.path if parsed.path else "/" path = path.rstrip("/") if not path: path = "index.html" elif not os.path.splitext(path)[1]: path = path + "/index.html" path = os.path.normpath(path) if path.startswith("/"): path = path[1:] return os.path.join(base_dir, path) ``` The resulting path is subsequently passed to file-writing operations: ```python page_dir = url_to_filepath(self.base_dir, url) if not page_dir.endswith(".html"): page_dir += ".html" rewritten = rewrite_html_content(html_content, url, os.path.dirname(page_dir)) self.save_file(page_dir, rewritten.encode("utf-8")) ``` ### Technical Analysis The URL path is normalized and has only one leading slash removed. On POSIX systems, `os.path.normpath()` may preserve exactly two leading slashes. For example, a URL path such as `//tmp/payload` can remain `//tmp/payload/index.html` after normalization. Removing only the first slash produces `/tmp/payload/index.html`. Because the second argument to `os.path.join()` is now absolute, Python discards `base_dir` and returns the attacker-influenced absolute path. No canonical-path containment check is performed before `save_file()` creates parent directories and writes the downloaded content. The same unsafe conversion function is used for both pages and resources. ### Attack Path 1. An attacker controls the target website or convinces a user or Agent to mirror a crafted URL. 2. The requested URL contains a path beginning with exactly two slashes, such as `https://attacker.example//tmp/payload`. 3. The server returns attacker-controlled conte ...[truncated 700 chars]
- Remediation
- ## Remediation Suggestions - Convert the mirror root and candidate destination to canonical absolute paths with `os.path.realpath()`. - Reject URL paths containing abnormal leading slashes, traversal components, null bytes, or platform-specific path separators. - Treat URL paths as relative components rather than removing only one leading slash. - Verify containment with `os.path.commonpath()` before creating directories or writing files. - Reject the destination if it equals or falls outside the intended mirror root. - Apply the containment check immediately before every write, not only during URL parsing. - Add regression tests for `//tmp/file`, `///tmp/file`, encoded traversal sequences, backslashes, absolute Windows paths, and symbolic-link escapes. A hardened pattern is: ```python root = os.path.realpath(base_dir) relative_path = path.lstrip("/\\") destination = os.path.realpath(os.path.join(root, relative_path)) if os.path.commonpath([root, destination]) != root: raise ValueError("Destination escapes the mirror directory") ```
