T09 · Insecure Skill Coding Practices
Warning
- Location
- health-data.sh:17
- Finding
- Unsafe Output-File Overwrite Through Symbolic-Link Following<![CDATA[ ## Vulnerability Details **File Location**: `health-data.sh`, lines 17–28 and 211–213 **Vulnerability Type**: Unsafe file creation, destructive overwrite, and symbolic-link following **Risk Level**: Medium ### Vulnerable Code ```bash prepare_output_file() { local target="$1" local dir dir=$(dirname "$target") mkdir -p "$dir" local prev_umask prev_umask=$(umask) umask 0077 : > "$target" umask "$prev_umask" chmod 600 "$target" } ``` The prepared path is subsequently reopened and truncated: ```bash if [[ -n "$output_path" ]]; then prepare_output_file "$output_path" exec 3>"$output_path" sink_fd=3 close_sink=1 fi ``` The output path originates from the user-controlled `--out` argument: ```bash --out) shift [[ $# -gt 0 ]] || die "--out requires a file path" out_path="$1" shift ;; ``` ### Technical Analysis The `--out` destination is opened with shell redirection without checking whether it already exists or is a symbolic link. The first `: > "$target"` operation follows symbolic links and truncates the resolved target. The subsequent `chmod 600 "$target"` also follows the link and changes the target's permissions. Finally, `exec 3>"$output_path"` reopens and truncates the same target. The use of a restrictive `umask` and `chmod 600` protects newly created files from broad read access, but it does not make path resolution safe. There is also a time-of-check/time-of-use concern if separate checks were added without atomic creation. ### Attack Path 1. An attacker identifies an output path that a user or automated process will pass to `export-json --out`. 2. The attacker creates a symbolic link at that path pointing to another file writable by the victim process. 3. The victim runs the command with the attacker-controlled or predictable output path. 4. `prepare_output_file` follows the symbolic link, truncates the target, and changes its permissions to mode `600`. 5. The target is opened and truncated again befor ...[truncated 708 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Reject symbolic links and refuse to overwrite existing destinations by default. - Create the destination atomically with exclusive-creation and no-follow semantics, equivalent to `O_CREAT | O_EXCL | O_NOFOLLOW`. - Prefer a small helper in a language that exposes secure file-opening flags; shell redirection alone cannot reliably provide all required guarantees. - If overwrite support is necessary, require an explicit option and securely validate the destination immediately before opening it. - Avoid the current create–`chmod`–reopen sequence. Open the file once with mode `0600` and retain that descriptor for the entire write. - Validate that the destination directory is trusted and not writable by untrusted users. - Do not run the skill with elevated privileges. For environments where only Bash is available, `set -o noclobber` can reduce accidental overwrites, but it should not be treated as a complete substitute for atomic `O_NOFOLLOW` file creation. ]]>