T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/run-mission-control-visual-qa.sh:14
- Finding
- Remote Command Injection Through Unquoted Environment-Controlled Paths<![CDATA[ ## Vulnerability Details **File Location**: `scripts/run-mission-control-visual-qa.sh`, lines 14–24 **Vulnerability Type**: OS command injection in remotely executed shell commands **Risk Level**: High ### Vulnerable Code ```bash ssh "${SSH_TARGET}" "mkdir -p ${REMOTE_RUN_DIR} ${OUTPUT_DIR}" scp "${LOCAL_SCRIPT_DIR}/${SCRIPT_NAME}" "${SSH_TARGET}:${REMOTE_RUN_DIR}/${SCRIPT_NAME}" quoted_args=() for arg in "$@"; do quoted_args+=("$(printf '%q' "$arg")") done # shellcheck disable=SC2029 ssh "${SSH_TARGET}" "cd ${REMOTE_RUN_DIR} && OUTPUT_DIR='${OUTPUT_DIR}' node ./${SCRIPT_NAME} ${quoted_args[*]}" ``` ### Technical Analysis The environment-controlled variables `REMOTE_RUN_DIR` and `OUTPUT_DIR` are inserted into command strings that SSH sends to a remote shell. They are not safely encoded as individual remote-shell arguments. On line 14, both variables are interpolated without quoting: ```bash ssh "${SSH_TARGET}" "mkdir -p ${REMOTE_RUN_DIR} ${OUTPUT_DIR}" ``` Consequently, shell operators, substitutions, redirections, whitespace, and other metacharacters contained in either variable are interpreted by the remote shell rather than treated exclusively as path characters. Line 24 has the same problem with `REMOTE_RUN_DIR`: ```bash ssh "${SSH_TARGET}" "cd ${REMOTE_RUN_DIR} && ..." ``` Although `OUTPUT_DIR` is surrounded by literal single quotes on that line, the value is interpolated before the command reaches the remote shell. An embedded single quote can terminate the intended quoted context and permit additional shell syntax. The local quoting of `SSH_TARGET` only protects the local shell. It does not protect values embedded within the command string interpreted by the remote shell. The URL arguments are processed with `printf '%q'`, but equivalent protection is not applied to the environment-controlled paths. ### Attack Path 1. An attacker gains the ability to influence the environment used to launch the helper script, such as through a ...[truncated 1353 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not concatenate environment-controlled values directly into remote shell command strings. 2. Restrict `REMOTE_RUN_DIR` and `OUTPUT_DIR` to approved absolute paths or approved subdirectories. Reject control characters, shell metacharacters, path traversal components, and unexpected prefixes. 3. Encode every value according to the remote shell's quoting rules. Local double quotes do not provide remote-shell argument safety. 4. Prefer a fixed remote runner that accepts validated positional arguments rather than dynamically constructing an entire shell command. 5. If shell command construction cannot be avoided, use a dedicated quoting function for every dynamic remote argument: ```bash shell_quote() { printf "'%s'" "$(printf '%s' "$1" | sed "s/'/'\\\\''/g")" } remote_run_dir_q="$(shell_quote "$REMOTE_RUN_DIR")" output_dir_q="$(shell_quote "$OUTPUT_DIR")" ssh -- "$SSH_TARGET" \ "mkdir -p -- ${remote_run_dir_q} ${output_dir_q}" ``` Apply equivalent quoting to the `cd` command and environment assignment. Also ensure URL arguments are passed as discrete, safely quoted values rather than relying on an array expansion embedded in a command string. 6. Prefer fixed server-side directories where practical, eliminating the need for these paths to be supplied through the environment. 7. Run the remote QA process under a dedicated, least-privileged account with access only to the required runner and output directories. 8. Add automated tests containing spaces, single quotes, command substitutions, separators, and newlines to verify that hostile path values are rejected or treated strictly as data. ]]>
