T09 · Insecure Skill Coding Practices
Error
- Location
- template-filling-execution.md:48
- Finding
- Fixed Heredoc Delimiter Allows Shell Command Injection## Vulnerability Details **File Location**: `template-filling-execution.md`, lines 48–64 **Vulnerability Type**: Shell command injection through premature heredoc termination **Risk Level**: High ### Vulnerable Code ```bash VALUES_FILE="$(mktemp /tmp/oa-values.XXXXXX.json)" chmod 600 "$VALUES_FILE" trap 'rm -f "$VALUES_FILE"' EXIT cat > "$VALUES_FILE" << 'FIELDS' { "field_name": "value" } FIELDS ``` The workflow instructs the agent to insert user-provided field values into a shell heredoc that uses the fixed delimiter `FIELDS`. Although quoting the delimiter prevents shell expansion within the heredoc body, it does not prevent a user-controlled line containing exactly `FIELDS` from terminating the heredoc. The validation rules explicitly permit tab and newline characters. Neither `template-filling-execution.md` nor the corresponding rules in `SKILL.md` reject a line equal to the fixed heredoc delimiter. The additional prohibition against common shell metacharacters is insufficient because ordinary command names, spaces, and arguments can form an executable payload without those prohibited characters. ### Technical Analysis Shell heredocs terminate when the shell encounters a line consisting only of the configured delimiter. If an agent constructs the documented command by directly embedding collected values, an attacker can provide a multiline contract field that closes the heredoc early. Text after the injected delimiter is parsed by the shell rather than written to the JSON file. The quoted form `<< 'FIELDS'` only controls expansion of text while it remains inside the heredoc; it offers no protection after premature termination. This is especially dangerous because the Skill is designed to process untrusted agreement terms and expressly allows multiline descriptions. The secure temporary filename and restrictive file permissions do not mitigate command injection during creation of that file. ### Attac ...[truncated 1343 chars]
- Remediation
- ## Remediation Suggestions 1. Do not serialize untrusted values by interpolating them into a shell heredoc. 2. Use a JSON-aware API in a language such as Node.js or Python to construct the values file. Pass user values as data rather than source code or generated shell text. 3. If shell use is unavoidable, provide serialized JSON through a non-evaluating channel and generate a cryptographically unpredictable heredoc delimiter. Verify that the delimiter does not occur as a complete line in the serialized content. 4. Prefer invoking the CLI through a process-spawning API with an argument array rather than constructing a shell command. 5. Validate all values against their documented length limits and reject unexpected control characters. Consider disallowing newlines in fields that do not require multiline input. 6. Add regression tests containing `FIELDS` on its own line, multiline values, quotes, backslashes, Unicode, and malformed JSON. 7. Retain the existing `mktemp`, permission restriction, and cleanup controls, as these remain useful for protecting confidential agreement data.