T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/run_full.py:24
- Finding
- Predictable Files in a Shared Temporary Directory Allow Symlink-Based File Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `scripts/run_full.py:24-29`, `scripts/parse_tables.py:177-178`, `scripts/build_diff_report.py:170`, and `scripts/mark_bill_diff.py:70` **Vulnerability Type**: Unsafe temporary-file handling and symlink-following file writes **Risk Level**: Medium ### Vulnerable Code `scripts/run_full.py:24-29` selects the shared `/tmp` directory by default and constructs predictable output names: ```python out_dir = os.path.abspath(sys.argv[3]) if len(sys.argv) > 3 else '/tmp' os.makedirs(out_dir, exist_ok=True) here = os.path.dirname(os.path.abspath(__file__)) parsed_json = os.path.join(out_dir, 'gtyt_parsed.json') diff_xls = os.path.join(out_dir, '两表金额差异对比.xls') ``` The generated JSON is opened for writing without exclusive creation or protection against symbolic links in `scripts/parse_tables.py:177-178`: ```python with open(out_path, 'w', encoding='utf-8') as f: json.dump(output, f, ensure_ascii=False, indent=2) ``` The generated report is saved directly to the predictable path in `scripts/build_diff_report.py:170`: ```python wb.save(out_path) ``` The marked bill is likewise saved directly to its predictable path in `scripts/mark_bill_diff.py:70`: ```python wbcopy.save(out_path) ``` ### Technical Analysis The workflow defaults to a globally writable directory and uses deterministic filenames. It does not create a private per-run directory, verify that destination paths are regular files, reject symbolic links, or use exclusive file creation. On systems where `/tmp` is writable by multiple users, another local user can create a symbolic link at one of the expected output paths before the workflow runs. Standard write operations generally follow symbolic links. Consequently, the process may truncate and overwrite the link target using the privileges of the account running the Skill. The vulnerable paths include: - `/tmp/gtyt_parsed.json` - `/tmp/两表金额差异对比.xls` - `/tmp/共同赢<month>账单-上海-已标差异.xls` The month ...[truncated 1889 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Use a private directory for every run.** Create it with `tempfile.mkdtemp()` or `tempfile.TemporaryDirectory()` rather than writing predictable files directly under `/tmp`: ```python import tempfile out_dir = ( os.path.abspath(sys.argv[3]) if len(sys.argv) > 3 else tempfile.mkdtemp(prefix='gtyt-reconcile-') ) os.chmod(out_dir, 0o700) ``` 2. **Avoid predictable temporary filenames.** Use `tempfile.NamedTemporaryFile` or randomized names inside the private directory. 3. **Reject symbolic-link destinations.** Before replacing a caller-selected output, use `os.lstat()` to detect symbolic links. Where supported, open files using `os.open()` with `O_NOFOLLOW`, `O_CREAT`, and `O_EXCL`. 4. **Write and publish atomically.** Generate each file in a private temporary directory, flush and close it, and then use `os.replace()` to publish it to an approved destination. Ensure the destination directory is trusted and not writable by untrusted users. 5. **Validate caller-supplied output directories.** Require the directory to be owned by the executing user and reject globally writable directories unless they have appropriate isolation and a private child directory is created. 6. **Run with least privilege.** The reconciliation process should not execute as `root` or another privileged service account. Restrict its filesystem permissions to the input and designated output directories. ]]>