T09 · Insecure Skill Coding Practices
Error
- Location
bin/gc-agent.sh:96- Finding
Shell Command Injection Through Unvalidated GC Configuration
- Content
View full analysis
/dev/null) echo "${val:-$default}" else echo "$default" fi } MAX_CP=$(get_gc_config "gc.max_checkpoints" 10) MAX_AGE=$(get_gc_config "gc.max_age_days" 7) REPORT_RETENTION=$(get_gc_config "gc.report_retention_count" 20) COMPRESS_TRIGGER=$(get_gc_config "gc.compress_trigger_lines" 200) TRASH_RETENTION=$(get_gc_config "gc.trash_retention_days" 30) ``` The resulting values are subsequently evaluated as Bash arithmetic expressions: ```bash local age_seconds=$((MAX_AGE * 86400)) ``` ```bash if [[ $cps_count -gt $MAX_CP ]]; then local excess=$(($cps_count - MAX_CP)) local to_delete=$(echo "$cps_list" | sort -t: -k2 | head -$excess) ``` ```bash local max_age_seconds=$((TRASH_RETENTION * 86400)) ``` ### Technical Analysis Numeric settings read from the workspace-controlled `.harness/config.json` file are not validated before they are inserted into Bash arithmetic contexts. Bash does not treat variable contents in arithmetic expansion strictly as decimal data. Instead, values may be recursively interpreted as arithmetic expressions. Crafted expressions, particularly expressions using array subscripts containing command substitutions, can therefore cause shell commands to execute when the GC process evaluates values such as `MAX_AGE`, `MAX_CP`, or `TRASH_RETENTION`. The affected code also uses unvalidated values in numeric comparisons and as the argument to `head`, increasing the likelihood of unexpected command behavior or denial of service even when a payload does not achieve comman ...[truncated 1450 chars]- Remediation
View remediation
max )); then log_error "$name is outside the permitted range" exit 1 fi } ``` 2. Apply validation immediately after configuration loading and before any arithmetic expansion: ```bash validate_uint "gc.max_checkpoints" "$MAX_CP" 0 10000 validate_uint "gc.max_age_days" "$MAX_AGE" 0 36500 validate_uint "gc.report_retention_count" "$REPORT_RETENTION" 0 100000 validate_uint "gc.compress_trigger_lines" "$COMPRESS_TRIGGER" 1 10000000 validate_uint "gc.trash_retention_days" "$TRASH_RETENTION" 0 36500 ``` 3. Reject invalid configuration rather than silently substituting or evaluating it. 4. Validate `DAEMON_INTERVAL` and all numeric command-line arguments using the same approach. 5. Use `10#$value` after validation to force decimal interpretation and avoid octal parsing. 6. Add tests containing whitespace, negative values, excessive values, arithmetic operators, array syntax, and command-substitution payloads. ]]>