T09 · Insecure Skill Coding Practices
Warning
- Location
xiaoclawshu.sh:87- Finding
Predictable Temporary File Enables Local File Clobbering and Avatar Data Exposure
- Content
View full analysis
/dev/null; then convert "$img_path" -resize 256x256 -quality 85 "$tmp_avatar" else cp "$img_path" "$tmp_avatar" fi avatar_b64=$(base64 -w0 "$tmp_avatar") rm -f "$tmp_avatar" ``` ### Technical Analysis The script constructs a temporary filename in the shared `/tmp` directory using only the current shell process ID: ```bash tmp_avatar="/tmp/xiaoclawshu_avatar_$$.jpg" ``` Process IDs are predictable and visible to other local users on many systems. The script does not create the file atomically, verify that the path is not a symbolic link, or place it inside a private directory. Consequently, another local process can pre-create the expected path as a symbolic link. Both `convert` and `cp` ordinarily follow symbolic links when writing to the destination. If the predicted path points to another file writable by the user invoking the Skill, the avatar operation can overwrite that target with image content. The script also performs cleanup only after successful conversion, copying, and Base64 encoding. Because `set -e` is enabled, a failure before `rm -f` can terminate the script and leave the temporary image in the globally accessible temporary directory. Its effective confidentiality then depends on the user's `umask` and file permissions applied by `convert` or `cp`. This temporary file is needed only to support optional avatar resizing; using a predictable shared path is not necessary for the Skill's declared functionality. ### Attack Path 1. A local attacker sharing the system monitors process creation or predicts the PID assigned to an upcoming invocation of the script. 2. The attacker creates `/tmp/xiaoclawshu_avatar_.jpg` as a s ...[truncated 1387 chars]- Remediation
View remediation
/dev/null 2>&1; then convert -- "$img_path" -resize 256x256 -quality 85 "$tmp_avatar" else cp -- "$img_path" "$tmp_avatar" fi chmod 600 "$tmp_avatar" avatar_b64=$(base64 -w0 "$tmp_avatar") ``` Additional hardening measures: 1. Respect `${TMPDIR:-/tmp}` rather than hardcoding `/tmp`. 2. Set a restrictive `umask`, such as `umask 077`, before creating temporary content. 3. Install the cleanup trap immediately after creating the private directory so failure paths are covered. 4. Use `--` before user-controlled file paths where supported to prevent paths beginning with `-` from being interpreted as options. 5. Validate the input as a regular, readable, non-symbolic-link file when the intended policy does not permit links. 6. Enforce the documented image size and type limits before transmitting the resulting data. ]]>