T09 · Insecure Skill Coding Practices
- Location
scripts/start_hotfix.sh:20- Finding
Unvalidated Incident ID Enables Arbitrary Path Traversal and File Overwrite
- Content
View full analysis
"docs/incidents/${ID}/TIMELINE.md" < "docs/incidents/${ID}/ROLLBACK.md" </dev/null 2>&1; then git status --short > "$OUT/git-status.txt" || true git log --oneline -n 30 > "$OUT/git-log.txt" || true git diff --stat > "$OUT/diff-stat.txt" || true git diff --name-only > "$OUT/changed-files.txt" || true fi ``` ### Technical Analysis Both scripts accept an incident identifier through `--id` and embed it directly in filesystem paths. Although the variables are quoted, quoting only prevents shell word splitting and glob expansion; it does not prevent path traversal through `../` components or absolute-path-like constructions. The scripts do not enforce the documented `INC-1234` format, canonicalize the resulting path, or verify that it remains beneath `docs/incidents/`. The sanitized `SLUG` variable is used only for the Git branch name and therefore does not protect the paths constructed from the original `ID`. The `start_hotfix.sh` script uses truncating redirections for predictable filenames, allowing existing writable files at resolved target paths to be overwritten. The evidence script similarly creates a destination controlled by the supplied identifier and writes Git ...[truncated 1490 chars]- Remediation
View remediation
&2 exit 1 fi ``` 2. Explicitly reject path separators, `.` components, control characters, and empty sanitized identifiers. 3. Resolve the intended root and output directory to canonical paths, then verify that the output remains beneath the incident root. 4. Use the validated or sanitized identifier consistently for both branch names and filesystem paths. 5. Avoid silently overwriting existing incident files. Use no-clobber behavior, test for existing files, or require explicit overwrite confirmation. 6. Consider restrictive directory permissions, such as `umask 077`, before creating incident evidence. 7. Add automated tests covering `../`, absolute paths, repeated separators, control characters, empty values, and valid incident identifiers. ]]>
