T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/submit_report.sh:132
- Finding
- Command Injection in the Single-Item Shell Helper<![CDATA[ ## Vulnerability Details **File Location**: `scripts/submit_report.sh:132-157`; identical vulnerable code is present in `openclaw-skill/scripts/submit_report.sh:132-157` **Vulnerability Type**: Shell command injection caused by malformed heredoc argument placement **Risk Level**: High ### Vulnerable Code ```bash payload="$( python3 - <<PY import json import sys payload = { "items": [ { "source_id": sys.argv[1], "title": sys.argv[2], "description": sys.argv[3], "collected_at": sys.argv[4], "agent_id": sys.argv[5], "agent_version": sys.argv[6], "source_type": sys.argv[7], } ] } lat = sys.argv[8] lng = sys.argv[9] if lat.strip() and lng.strip(): payload["items"][0]["lat"] = float(lat) payload["items"][0]["lng"] = float(lng) print(json.dumps(payload, ensure_ascii=False, separators=(",", ":"))) PY "${SOURCE_ID}" "${TITLE}" "${DESCRIPTION}" "${COLLECTED_AT}" "${AGENT_ID}" "${AGENT_VERSION}" "${SOURCE_TYPE}" "${LAT}" "${LNG}" )" ``` The coordinate-rounding blocks at `scripts/submit_report.sh:109-121` and the corresponding packaged copy use the same malformed pattern. ### Technical Analysis Arguments for a command that consumes a heredoc must appear on the command line before the heredoc redirection. In this implementation, the heredoc terminator ends the `python3 -` command, and the following expanded values are parsed by Bash as a separate command. As a result: 1. The Python process does not receive the expected `sys.argv[1:]` values and normally raises an `IndexError`. 2. In ordinary non-POSIX Bash configurations, `errexit` is not inherited inside command substitutions. Execution can therefore continue after the Python failure. 3. Bash treats the attacker-controlled `SOURCE_ID` value as the executable name and the remaining user-controlled fields as its arguments. This permits arbitrary command execution in common Bash configurations. It also makes the documented single-item helper functio ...[truncated 1454 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Move all Python arguments before the heredoc redirection and quote the heredoc delimiter: ```bash payload="$( python3 - \ "$SOURCE_ID" \ "$TITLE" \ "$DESCRIPTION" \ "$COLLECTED_AT" \ "$AGENT_ID" \ "$AGENT_VERSION" \ "$SOURCE_TYPE" \ "$LAT" \ "$LNG" <<'PY' import json import sys payload = { "items": [{ "source_id": sys.argv[1], "title": sys.argv[2], "description": sys.argv[3], "collected_at": sys.argv[4], "agent_id": sys.argv[5], "agent_version": sys.argv[6], "source_type": sys.argv[7], }] } lat = sys.argv[8] lng = sys.argv[9] if lat.strip() and lng.strip(): payload["items"][0]["lat"] = float(lat) payload["items"][0]["lng"] = float(lng) print(json.dumps(payload, ensure_ascii=False, separators=(",", ":"))) PY )" ``` Apply the same correction to both coordinate-rounding blocks: ```bash LAT="$( python3 - "$LAT" "$APPROX_DECIMALS" <<'PY' import sys print(round(float(sys.argv[1]), int(sys.argv[2]))) PY )" ``` Additional hardening should include: - Applying the fix to both copies of the shell helper. - Validating latitude, longitude, decimal count, source type, timestamp, and identifier lengths before use. - Adding shell regression tests with values such as `/bin/sh`, `-c`, spaces, newlines, command substitutions, and shell metacharacters. - Running `shellcheck` in continuous integration. - Preferably replacing the duplicate shell implementation with a call to the already structured Python implementation to reduce attack surface. ]]>