T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/script.sh:250- Finding
Path Traversal Allows File Overwrite Outside the Story Directory
- Content
View full analysis
"$DATA_DIR/stories/$name.md" echo " Saved: $name" _log "save" "$name" } ``` ### Technical Analysis The `cmd_save` function directly incorporates the user-controlled `name` argument into a filesystem path. Although the variable is quoted, quoting only prevents shell word splitting and expansion; it does not prevent path traversal sequences such as `../`. The destination is constructed as: ```text $DATA_DIR/stories/.md ``` An attacker can supply enough parent-directory components to escape `$DATA_DIR/stories` and select another writable file ending in `.md`. The redirection operator truncates an existing destination before writing standard input, so this can overwrite data outside the intended story-storage directory. The automatically appended `.md` suffix limits direct targeting to paths whose resulting filename ends in `.md`. Nevertheless, documentation, application data, repository files, and other Markdown files writable by the invoking user remain exposed. The function also follows symbolic links, which can increase the overwrite scope if a malicious or compromised local process can place a symlink in the stories directory. ### Attack Path 1. Determine or infer the configured data directory. By default it is: ```text $HOME/.local/share/story-writer ``` 2. Choose a writable target ending in `.md`, such as a Markdown file in another project directory. 3. Calculate a relative traversal path from: ```text $DATA_DIR/stories ``` to the target, omitting the final `.md` because the function appends it automatically. 4. Invoke the save command with the traversal sequence: ```bash printf '%s\n' 'attacker-controlled replacement content' | bash scripts/ ...[truncated 1152 chars]- Remediation
View remediation
&2 return 1 } ``` 2. Reject absolute paths, path separators, `.` and `..` components, control characters, and empty names. 3. Construct and canonicalize the destination, then verify that it remains beneath the canonical stories directory: ```bash stories_dir="$(realpath "$DATA_DIR/stories")" destination="$stories_dir/$name.md" resolved_parent="$(realpath -m "$(dirname "$destination")")" [[ "$resolved_parent" == "$stories_dir" ]] || { echo "Destination escapes the stories directory" >&2 return 1 } ``` 4. Prevent unintended truncation by using non-clobbering creation when overwriting is not explicitly requested: ```bash set -o noclobber cat > "$destination" ``` 5. Defend against symbolic-link targets. Check that the destination is not a symlink and, where available, use a secure file-opening mechanism with `O_NOFOLLOW`. 6. Consider separating the display title from the filesystem identifier. Generate a safe internal filename rather than treating an arbitrary story title as a path component. ]]>