T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/ari.py:1474
- Finding
- Export Function Allows Unrestricted File Overwrite and Symlink Following<![CDATA[ ## Vulnerability Details **File Location**: `scripts/ari.py`, lines 1474–1475 **Vulnerability Type**: Arbitrary file overwrite through an attacker-influenced export path **Risk Level**: Medium ### Vulnerable Code ```python with open(dest, "wb") as fh: fh.write(body) ``` The destination is derived directly from the `--out` command-line argument: ```python dest = args.out or ("ari_report_%d.%s" % ( args.report_id, "html" if fmt == "html" else "md" )) emit(request_download( "/api/v1/export/reports/%d" % args.report_id, {"format": fmt}, dest ), args.compact) ``` The same behavior applies to review exports: ```python dest = args.out or ("ari_reviews_%s.csv" % args.asin.upper()) emit(request_download( "/api/v1/export/reviews", {"asin": args.asin.upper(), "site": args.site}, dest ), args.compact) ``` ### Technical Analysis The export handler accepts an unrestricted path through `--out` and opens it using Python's `"wb"` mode. This mode creates a file when it does not exist and truncates an existing file before writing. The implementation does not: - Restrict exports to a dedicated directory. - Reject absolute paths or parent-directory traversal. - Check whether the destination already exists. - Detect symbolic links. - Use `O_NOFOLLOW` where supported. - Create the file exclusively with `O_EXCL`. - Ask for confirmation before replacing an existing file. Consequently, any file writable by the account running the Skill can be replaced with server-provided export content. Symbolic links are followed by the regular `open()` call, producing a time-of-check/time-of-use and link-following risk in shared or attacker-controlled directories. This flaw is classified as `T09: Insecure Skill Coding Practices`. It does not independently provide privilege escalation: file-system access remains constrained by the operating-system privileges of the Skill process. ### Attack Path A practical exploitation sequence is: 1. An atta ...[truncated 2118 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Use a dedicated export directory by default** Resolve all generated exports beneath a private user directory, such as `~/.ari/exports`, and create it with restrictive permissions. 2. **Require explicit opt-in for arbitrary paths** Treat `--out` as a potentially destructive option. Reject absolute paths and paths that resolve outside the designated export directory unless the user supplies a separate explicit override. 3. **Do not overwrite existing files by default** Open new files atomically and exclusively: ```python flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL if hasattr(os, "O_NOFOLLOW"): flags |= os.O_NOFOLLOW fd = os.open(dest, flags, 0o600) with os.fdopen(fd, "wb") as fh: fh.write(body) ``` Return an error if the destination already exists. If overwrite support is required, add a clearly documented `--force` option and require explicit user confirmation. 4. **Reject symbolic links and non-regular destinations** Validate the parent directory and destination with `os.lstat()`. Reject symbolic links, device files, FIFOs, sockets, and other non-regular files. 5. **Canonicalize and validate the destination** Resolve the destination and approved export directory with `pathlib.Path.resolve()`, then verify that the destination remains inside the approved directory. 6. **Use atomic replacement only after validation** Download to a newly created temporary file in the same trusted directory, flush and synchronize it, and then rename it atomically. Do not place temporary files in shared directories. 7. **Run with least privilege** Document that the CLI must not be run as root or another privileged service account. This limits the damage possible from path manipulation or accidental overwrites. ]]>