T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/wiki_entry_step_checkpoint.sh:94
- Finding
- Arbitrary Shell Command Execution Through Checkpoint Audit Commands<![CDATA[ ## Vulnerability Details **File Location**: `scripts/wiki_entry_step_checkpoint.sh:94-101` **Vulnerability Type**: OS command injection through dynamic shell evaluation **Risk Level**: High ### Vulnerable Code ```bash if [ "$STATUS" = "done" ]; then prev=1 while [ "$prev" -lt "$STEP" ]; do s=$(get_status "$prev") if [ "$s" != "done" ]; then echo "❌ 防跳步: Step $STEP 不能标记 done，因为 Step $prev 当前是 $s" exit 1 fi prev=$((prev + 1)) done if [ -n "$AUDIT_CMD" ]; then echo "[micro-audit] $AUDIT_CMD" bash -lc "$AUDIT_CMD" rc=$? if [ "$rc" -ne 0 ]; then echo "❌ micro-audit 失败，Step $STEP 不能标记 done" exit 2 fi else echo "⚠️ Step $STEP 标记 done 但未提供 --audit-cmd" fi fi ``` ### Technical Analysis The `--audit-cmd` argument is accepted as an unrestricted string and passed directly to: ```bash bash -lc "$AUDIT_CMD" ``` This invokes a login shell and interprets all shell metacharacters, substitutions, pipelines, redirections, and compound commands contained in the argument. Quoting the variable at this invocation does not make the command safe because `bash -c` intentionally parses its contents as shell syntax. The interface is more powerful than required for the documented micro-audit operations. The examples only require fixed checks such as testing that a file exists or searching for a known marker, but the implementation permits arbitrary commands. If an untrusted note, generated workflow value, prompt, or operator-controlled input influences the audit command, it can convert a checkpoint operation into arbitrary code execution. ### Attack Path 1. An attacker causes malicious text to influence the audit operation selected by the Agent. 2. The resulting command is supplied to the checkpoint script, for example: ```bash --audit-cmd "legitimate_check; attacker_command" ``` 3. The script reaches a `done` checkpoint after previous steps are marked complete. 4. `bash -lc` parses a ...[truncated 955 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove the free-form `--audit-cmd` interface. 2. Replace it with fixed audit types, such as: ```bash --audit-type file-exists --path "Knowledge/example.md" --audit-type contains-fixed --path "Knowledge/example.md" --value "marker" ``` 3. Invoke utilities directly with argument arrays rather than through `bash -c`: ```bash test -f "$validated_path" grep -Fq -- "$expected_value" "$validated_path" ``` 4. Canonicalize every supplied path and verify that it remains inside the configured vault. 5. Use an explicit allowlist of supported audit operations. 6. Reject shell metacharacters only as defense in depth; do not depend on filtering as the primary fix. 7. Run checkpoint validation with the minimum filesystem and network privileges available. 8. Add regression tests proving that values containing `;`, `|`, `$()`, backticks, redirections, and newlines are treated as data and never executed. ]]>