T09 · Insecure Skill Coding Practices
Error
- Location
scripts/download_log.sh:64- Finding
Remote Command Injection Through Unvalidated Module Names
- Content
View full analysis
/dev/null 2>&1; then mkdir -p "${SAVE_DIR}/${module}" scp -r ${REMOTE_USER}@${REMOTE_IP}:${REMOTE_LOG_DIR}/${module}/* "${SAVE_DIR}/${module}/" 2>/dev/null ``` ### Technical Analysis Every positional argument not beginning with a hyphen is appended directly to `MODULES`. The script defines a list of supported modules in `ALL_MODULES`, but it never verifies user-supplied module names against that list or applies a strict identifier validation rule. The value of `module` is interpolated into a command string passed to SSH: ```sh ssh root@ "test -d /app/apa/log/${module}" ``` Although the outer command string is quoted for the local shell, SSH sends the resulting string to a shell on the remote host. Shell metacharacters contained in `module`, such as a command separator, can consequently change the structure of the remote command rather than being treated as part of a directory name. The script uses the hard-coded `root` account for the SSH connection. Therefore, successful command injection executes with full administrative privileges on the target J6B board. The same untrusted v ...[truncated 1684 chars]- Remediation
View remediation
&2 exit 1 fi done ``` 2. **Apply strict syntactic validation as defense in depth** Only accept expected identifier characters: ```sh case "$module" in ''|*[!A-Za-z0-9_]*) echo "Invalid module name" >&2 exit 1 ;; esac ``` Syntactic validation should supplement, not replace, the fixed allowlist. 3. **Avoid interpolating input into remote shell programs** Use a fixed remote command and pass the validated value as a positional parameter: ```sh ssh "${REMOTE_USER}@${REMOTE_IP}" \ 'module=$1; test -d "/app/apa/log/$module"' sh "$module" ``` This approach still requires strict validation because remote argument transport can vary between SSH implementations. 4. **Quote all connection and path operands** Quote the SSH destination and local operands consistently. Where supported, use `--` to terminate command options and prefer an SCP/SFTP implementation that does not invoke a remote shell. 5. **Apply least privilege** Replace direct root access with a dedicated account that has read-only access to `/app/apa/log`. If archive creation is necessary, expose a narrowly scoped privileged helper rather than an unrestricted root shell. 6. **Harden SSH access** Restrict the diagnostic account using SSH key policies, source-address restrictions, forced commands, and disabled interactive shell access where operationally feasible. 7. **Add negative security tests** ...[truncated 201 chars]