T09 · Insecure Skill Coding Practices
- Location
scripts/lead-tracker.sh:17- Finding
Path Traversal Allows Unintended Markdown File Overwrite or Modification
- Content
View full analysis
Vulnerability Details
File Location:
scripts/lead-tracker.sh, lines 17–22 and 71–79
Vulnerability Type: Path traversal through an insufficiently sanitized company name
Risk Level: MediumVulnerable Code
The
addcommand constructs a file path directly from the user-controlled company name and then overwrites that path:bash # Create filename from company name filename=$(echo "$2" | tr '[:upper:]' '[:lower:]' | tr ' ' '-') lead_file="$LEADS_DIR/${filename}.md" cat > "$lead_file" << EOF # Lead: $2The
updatecommand repeats the same unsafe path construction and appends data to the selected file:bash filename=$(echo "$2" | tr '[:upper:]' '[:lower:]' | tr ' ' '-') lead_file="$LEADS_DIR/${filename}.md" if [ -f "$lead_file" ]; then echo "| $(date +%Y-%m-%d) | Stage update | Moved to: $4 |" >> "$lead_file" echo "✅ Updated $2 to stage: $4" else echo "❌ Lead not found: $2" fiTechnical Analysis
The company name in
$2is treated as untrusted input. The transformation only converts uppercase characters to lowercase and spaces to hyphens. It does not remove or reject:- Forward slashes
..parent-directory components- Absolute paths
- Repeated separators
- Symbolic-link destinations
As a result, a value such as
../pipeline/currentproduces:text $LEADS_DIR/../pipeline/current.mdAfter filesystem path resolution, this points to the pipeline file rather than a file beneath the leads directory.
The
addoperation uses the shell redirection operator>, so the targeted Markdown file is truncated and replaced. Theupdateoperation uses>>, so attacker-controlled stage data is appended to an existing Markdown file.The
.mdsuffix limits direct targeting to Markdown files. However, the affected application deliberately stores operational data and templates in Markdown, and other agent environments may also interp ...[truncated 2352 chars]- Remediation
View remediation
Remediation Suggestions
-
Validate company-derived filenames with a strict allowlist. Permit only lowercase ASCII letters, digits, and single hyphens:
bash filename=$(printf '%s' "$2" | tr '[:upper:]' '[:lower:]' | sed -E 's/[^a-z0-9]+/-/g; s/^-+//; s/-+$//') if [ -z "$filename" ]; then echo "Invalid company name" >&2 exit 1 fi -
Explicitly reject path separators, parent-directory references, and absolute paths before constructing the destination:
bash case "$2" in *"/"*|*".."*) echo "Company name contains prohibited path characters" >&2 exit 1 ;; esac -
Canonicalize the destination and verify that it remains beneath the canonical leads directory. The containment check should be performed after resolving existing parent directories and before opening the file.
-
Use one shared filename-validation function for both
addandupdateso the operations cannot apply inconsistent security rules. -
Prevent accidental destruction of existing records. Use noclobber behavior or explicitly refuse to overwrite a destination unless the user supplies a deliberate replacement option:
bash if [ -e "$lead_file" ]; then echo "Lead already exists: $lead_file" >&2 exit 1 fi -
Consider rejecting symbolic links and checking the destination immediately before writing. Where stronger guarantees are required, use a language or API capable of opening files relative to a trusted directory without following links.
-
Add regression tests for inputs including
../pipeline/current, absolute paths, repeated separators, empty sanitized names, leading dots, and symbolic-link destinations.
-