T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/ari.py:1474
- Finding
- Arbitrary Local File Overwrite Through the Export Output Path<![CDATA[ ## Vulnerability Details **File Location**: `scripts/ari.py:1474-1475`, with attacker-controlled path propagation at `scripts/ari.py:1590-1604` and argument definition at `scripts/ari.py:1903-1908` **Vulnerability Type**: Unrestricted file write and arbitrary file overwrite **Risk Level**: Medium ### Vulnerable Code ```python def request_download(path, params, dest): """Download a non-JSON response (CSV / HTML / Markdown) to a local file.""" query = {"method": "GET", "path": path, "params": {k: v for k, v in (params or {}).items() if v not in (None, "")}, "payload": None} url = base_url() + path if query["params"]: url += "?" + urllib.parse.urlencode(query["params"], doseq=True) headers = {"Authorization": "Bearer " + require_key(), "User-Agent": user_agent()} try: req = urllib.request.Request(url, headers=headers, method="GET") with urllib.request.urlopen(req, timeout=TIMEOUT_SEC) as resp: note_release(resp.headers) ctype = resp.headers.get("Content-Type", "") body = resp.read() if "application/json" in ctype: out = json.loads(body.decode("utf-8", errors="replace")) if isinstance(out, dict): out["_query"] = query return out tail = body[-300:].decode("utf-8", errors="replace") if "# export error:" in tail: return error_obj("ARI_EXPORT_ERROR", 200, tail.split("# export error:", 1)[1].strip(), "Export failed before completion; the incomplete file was not written.", query) with open(dest, "wb") as fh: fh.write(body) ``` The destination is derived directly from the command-line argument: ```python def cmd_export(args): """Export reviews or reports to a local file.""" if args.report_id: ...[truncated 4381 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Use a dedicated export directory** Resolve all default and user-selected output paths under a controlled directory such as `~/.ari/exports`: ```python export_root = os.path.realpath(os.path.expanduser("~/.ari/exports")) os.makedirs(export_root, mode=0o700, exist_ok=True) ``` 2. **Validate the resolved destination** Reject destinations that resolve outside the approved directory: ```python candidate = os.path.realpath(os.path.join(export_root, requested_name)) if os.path.commonpath([export_root, candidate]) != export_root: raise ValueError("Export path escapes the approved export directory") ``` 3. **Refuse silent replacement** Create new files exclusively rather than truncating existing files: ```python flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL if hasattr(os, "O_NOFOLLOW"): flags |= os.O_NOFOLLOW fd = os.open(candidate, flags, 0o600) with os.fdopen(fd, "wb") as fh: fh.write(body) ``` 4. **Reject symbolic links** Use `O_NOFOLLOW` where available and perform an `lstat`-based check on platforms that do not support it. Security-sensitive code should account for time-of-check/time-of-use races rather than relying solely on a preliminary path check. 5. **Write atomically** Write the response to a securely created temporary file in the approved destination directory, flush and synchronize it, and then rename it atomically. Do not replace an existing destination unless the user has explicitly requested overwrite behavior. 6. **Require explicit overwrite confirmation** If overwriting is a necessary feature, add a separate `--overwrite` option and clearly display the resolved target path before performing the write. Agent instructions should require direct user approval before using this option. 7. **Prefer filenames rather than unrestricted paths** Consider changing `--out` to accept only a filename. If arbitrary paths must remain ...[truncated 107 chars]