T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/add-invoice.sh:92- Finding
jq Program Injection Through an Unescaped Invoice Number
- Content
View full analysis
/dev/null; then echo "Error: Invoice number '$INV_NUMBER' already exists." exit 1 fi # ... # --- SAVE TO FILE --- temp_file=$(mktemp) jq ".invoices[\"$INV_NUMBER\"] = $invoice_json" "$INVOICES_FILE" > "$temp_file" && mv "$temp_file" "$INVOICES_FILE" ``` ### Technical Analysis The `INV_NUMBER` value is obtained from the user-controlled `--number` command-line argument and interpolated directly into two jq programs. Quoting the shell variable prevents shell word splitting, but it does not make the value safe for inclusion in jq source code. An invoice number containing a quote, closing bracket, and jq operators can terminate the intended property lookup and inject additional jq expressions. The first vulnerable invocation is used for duplicate detection, while the second transforms the database and writes the result back to `invoices.json`. For example, an invoice number shaped like: ```text x"] = {} | .invoices["attacker ``` changes the duplicate-check filter into an expression equivalent to: ```jq .invoices["x"] = {} | .invoices["attacker"] ``` and changes the database update into an expression equivalent to: ```jq .invoices["x"] = {} | .invoices["attacker"] = ``` This can bypass the intended unique-key behavior and modify records other than the literal invoice number supplied by the caller. More generally, arbitrary jq filters can be introduced, allowing an attacker to delete, replace, or restructure data in the invoice database. This is jq-language injection rather than direct shell command injection. The demonstrated code does not provide operating-system command execution, because the injected content is interpreted ...[truncated 1895 chars]- Remediation
View remediation
/dev/null; then echo "Error: Invoice number '$INV_NUMBER' already exists." exit 1 fi # Safe database update temp_file=$(mktemp) if jq \ --arg number "$INV_NUMBER" \ --argjson invoice "$invoice_json" \ '.invoices[$number] = $invoice' \ "$INVOICES_FILE" > "$temp_file"; then mv -- "$temp_file" "$INVOICES_FILE" else rm -f -- "$temp_file" echo "Error: Failed to update invoice database." >&2 exit 1 fi ``` Apply additional defense-in-depth validation to invoice numbers. If business requirements permit, restrict them to a documented allowlist such as letters, digits, periods, underscores, and hyphens: ```bash if [[ ! "$INV_NUMBER" =~ ^[A-Za-z0-9._-]{1,100}$ ]]; then echo "Error: Invalid invoice number format." >&2 exit 1 fi ``` Additional hardening should include: 1. Validate that the existing database has the expected object structure before updating it. 2. Validate all numeric and date inputs before creating the invoice object. 3. Remove the temporary file through a cleanup trap if the script exits before `mv`. 4. Add regression tests using invoice numbers containing quotes, brackets, pipes, backslashes, and jq operators. 5. Consider file locking around the read-check-write sequence to prevent concurrent invocations from losing updates. ]]>