T09 · Insecure Skill Coding Practices
- Location
workflow/step4-write.sh:18- Finding
Arbitrary Command Execution Through Unvalidated Chapter-Range Arithmetic
- Content
View full analysis
"$OUTPUT_FILE" done ``` Equivalent unvalidated parsing is used in `step3-plot.sh` and `step5-review.sh`. The full-pipeline script forwards the supplied range to all three scripts without validation: ```bash PROJECT_DIR="$1" CHAPTER_RANGE="${2:-1-10}" "$SCRIPT_DIR/step3-plot.sh" "$PROJECT_DIR" "$CHAPTER_RANGE" "$SCRIPT_DIR/step4-write.sh" "$PROJECT_DIR" "$CHAPTER_RANGE" "$SCRIPT_DIR/step5-review.sh" "$PROJECT_DIR" "$CHAPTER_RANGE" ``` ### Technical Analysis `CHAPTER_RANGE` is accepted directly from a command-line argument. Its two components are extracted with `cut`, but neither component is validated as a decimal integer before being used in Bash arithmetic contexts. Bash recursively interprets variable values referenced in arithmetic expressions. Consequently, an attacker-controlled value containing arithmetic syntax, array subscripts, or command substitution can be evaluated when the scripts process expressions such as: ```bash for ((i=START_CHAPTER; i<=END_CHAPTER; i++)) ``` Quoting the original command-line argument does not prevent this issue because the dangerous interpretation occurs later, inside the arithmetic evaluator. An illustrative malicious chapter-range argume ...[truncated 2068 chars]- Remediation
View remediation
&2 exit 1 fi START_CHAPTER=$((10#$START_RAW)) END_CHAPTER=$((10#$END_RAW)) if (( START_CHAPTER < 1 || END_CHAPTER < START_CHAPTER )); then echo "Invalid chapter range: END must be greater than or equal to START." >&2 exit 1 fi MAX_CHAPTERS_PER_RUN=100 if (( END_CHAPTER - START_CHAPTER + 1 > MAX_CHAPTERS_PER_RUN )); then echo "Chapter range exceeds the maximum permitted batch size." >&2 exit 1 fi ``` Apply this validation consistently in: - `workflow/run-full-pipeline.sh` - `workflow/step3-plot.sh` - `workflow/step4-write.sh` - `workflow/step5-review.sh` Additional hardening measures: 1. Validate at both the pipeline entry point and each independently executable step script. 2. Replace the `echo | cut` parsing logic with the regular-expression captures shown above. 3. Never place unvalidated external input in Bash arithmetic contexts. 4. Quote all ordinary variable expansions to prevent word splitting and pathname expansion. 5. Enforce reasonable upper bounds on chapter numbers and batch size to prevent excessive loops, disk consumption, and denial of service. 6. Add negative tests using arithmetic syntax, command substitutions, whitespace, glob characters, empty range components, reversed ranges, and excessively large numbers. 7. Consider enabling stricter shell behavior with `set -euo pipefail`, while recognizing that strict mode does not replace input validation. ]]>
