T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/archive_session.py:102
- Finding
- Path Traversal Through Unvalidated Session and Date Identifiers## Vulnerability Details **File Location**: `scripts/archive_session.py:102-113, 134-135` **Vulnerability Type**: Path traversal and arbitrary file overwrite **Risk Level**: High ### Vulnerable Code ```python def archive_session(session_id, messages, channel="webchat", date=None): """归档一个 session""" if date is None: date = datetime.now().strftime("%Y-%m-%d") year_month = date[:7] # YYYY-MM # 确保目录存在 session_dir = os.path.join(ARCHIVE_DIR, "sessions", year_month) os.makedirs(session_dir, exist_ok=True) archive_file = os.path.join(session_dir, f"{session_id}.json") # ... # 写入归档 with open(archive_file, "w", encoding="utf-8") as f: json.dump(archive, f, ensure_ascii=False, indent=2) ``` ### Technical Analysis The `date` and `session_id` parameters are incorporated into filesystem paths without format validation, canonicalization, or containment checks. A `session_id` containing `../` path segments can cause `archive_file` to resolve outside the intended session directory. An absolute `session_id` is especially dangerous because `os.path.join()` discards preceding path components when a later component is absolute. Similarly, the first seven characters of `date` are treated as a directory name without verifying that the value follows the expected `YYYY-MM-DD` format. The destination is opened with mode `"w"`, so an existing writable file is truncated before the archive JSON is written. Exploitation depends on an attacker being able to influence arguments supplied to `archive_session()`. The current script does not expose a command-line invocation for this function, but the Skill documentation presents it as an externally callable archival tool. ### Attack Path 1. An attacker gains control over, or influences, the `session_id` or `date` passed to `archive_session()`. 2. The attacker supplies a traversal identifier such as `../../ta ...[truncated 1120 chars]
- Remediation
- ## Remediation Suggestions 1. Enforce a strict allowlist for session identifiers, such as `^[A-Za-z0-9_-]{1,128}$`. 2. Parse dates with `datetime.strptime(date, "%Y-%m-%d")` and derive the directory from the parsed date rather than slicing untrusted input. 3. Resolve the archive root and destination with `pathlib.Path.resolve()`. 4. Verify that the resolved destination is a descendant of the intended session directory before opening it. 5. Reject absolute paths, path separators, `.` components, and `..` components in identifiers. 6. Consider atomic writes through a securely created temporary file followed by `os.replace()`. 7. If archives must not overwrite existing sessions, use exclusive creation mode (`"x"`) or explicitly handle duplicates. 8. Add tests covering absolute paths, traversal sequences, mixed separators, malformed dates, and symbolic-link edge cases. Example hardening approach: ```python from datetime import datetime from pathlib import Path import re SESSION_ID_PATTERN = re.compile(r"^[A-Za-z0-9_-]{1,128}$") def get_archive_path(session_id, date): if not SESSION_ID_PATTERN.fullmatch(session_id): raise ValueError("Invalid session identifier") parsed_date = datetime.strptime(date, "%Y-%m-%d") archive_root = ( Path(ARCHIVE_DIR) / "sessions" / parsed_date.strftime("%Y-%m") ).resolve() archive_root.mkdir(parents=True, exist_ok=True) destination = (archive_root / f"{session_id}.json").resolve() if destination.parent != archive_root: raise ValueError("Archive path escapes the archive directory") return destination ```