T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/coda.sh:50
- Finding
- Unsafe JSON Construction from Untrusted Command-Line Arguments<![CDATA[ ## Vulnerability Details **File Location**: `scripts/coda.sh`, lines 50–51, 151, and 157 **Vulnerability Type**: Improper JSON escaping and insufficient input validation **Risk Level**: Medium ### Vulnerable Code At lines 50–51, document properties are inserted directly into a JSON string: ```bash body="{\"title\":\"$1\"}" [[ -n "${2:-}" ]] && body="{\"title\":\"$1\",\"folderId\":\"$2\"}" ``` At line 151, folder properties are handled in the same way: ```bash _post -d "{\"name\":\"$1\",\"workspaceId\":\"$2\"}" "$BASE/folders" ``` At line 157, permission properties are directly interpolated: ```bash _post -d "{\"access\":\"$3\",\"principal\":{\"type\":\"email\",\"email\":\"$2\"}}" "$BASE/docs/$1/acl/permissions" ``` ### Technical Analysis The script constructs JSON by embedding command-line arguments directly between JSON quotation marks. It does not apply JSON-aware escaping to quotation marks, backslashes, control characters, or structural characters supplied in those arguments. An argument containing a quotation mark and JSON syntax can terminate the intended string value and introduce or modify object properties. Depending on the Coda API's JSON parser, schema validation, and treatment of duplicate properties, this can result in malformed requests or request semantics that differ from the command invocation presented to the user. The permission operation is particularly sensitive because both the requested access level and recipient email are inserted without escaping or validation. The script also does not enforce the documented `readonly`, `write`, or `comment` allowlist before submitting the request. This is JSON/data injection rather than shell-command injection. The interpolated content remains an argument passed to `curl`, and the reviewed code does not use `eval` or execute the resulting JSON as shell syntax. ### Attack Path 1. An attacker influences a document title, folder name, workspace identifier, recipient email, access v ...[truncated 1785 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Construct every request body with a JSON-aware serializer instead of manual string interpolation. For example: ```bash body="$(jq -n --arg title "$1" '{title: $title}')" if [[ -n "${2:-}" ]]; then body="$(jq -n \ --arg title "$1" \ --arg folderId "$2" \ '{title: $title, folderId: $folderId}')" fi _post -d "$body" "$BASE/docs" ``` Apply the same approach to folder creation: ```bash body="$(jq -n \ --arg name "$1" \ --arg workspaceId "$2" \ '{name: $name, workspaceId: $workspaceId}')" _post -d "$body" "$BASE/folders" ``` For document sharing, validate the access value and serialize all fields safely: ```bash case "$3" in readonly|write|comment) ;; *) echo "Error: access must be readonly, write, or comment." >&2 exit 1 ;; esac body="$(jq -n \ --arg access "$3" \ --arg email "$2" \ '{access: $access, principal: {type: "email", email: $email}}')" _post -d "$body" "$BASE/docs/$1/acl/permissions" ``` Additional hardening measures should include: 1. Validate resource identifiers against the documented Coda identifier format where practical. 2. Validate email addresses before permission mutations. 3. Reject control characters and unreasonable argument lengths. 4. Require explicit confirmation before destructive or access-control operations when the script is used interactively. 5. Add regression tests using quotation marks, backslashes, newlines, and JSON structural characters in every text argument. ]]>