T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/approval_gate.sh:66
- Finding
- Approval Verification Can Be Bypassed with Regular-Expression Input<![CDATA[ ## Vulnerability Details **File Location**: `scripts/approval_gate.sh`, lines 66–80 **Vulnerability Type**: Improper input handling in an authorization control **Risk Level**: High ### Vulnerable Code ```bash if [ -n "$ID" ]; then if grep -q "\"id\": *\"$ID\"" "$FILE" 2>/dev/null; then echo "approval_status=found id=$ID" exit 0 else echo "approval_status=missing id=$ID" exit 1 fi elif [ -n "$SCOPE" ]; then if grep -q "\"scope\": *\"$SCOPE\"" "$FILE" 2>/dev/null; then echo "approval_status=found scope=$SCOPE" exit 0 else echo "approval_status=missing scope=$SCOPE" exit 1 fi ``` ### Technical Analysis The script embeds the caller-controlled `ID` or `SCOPE` directly into a basic regular expression passed to `grep`. Regular-expression metacharacters are not escaped, and the JSONL file is not parsed as JSON. An input such as `.*` can therefore match the value of an unrelated approval record. The check also verifies only the requested ID or scope and does not bind the approval to other security-relevant properties such as risk, approver, operator, rollback owner, expiration, or the exact proposed action. This undermines the package's approval gate because a successful textual match is reported as a valid approval even when no exact approval exists for the requested operation. ### Attack Path 1. Obtain access to invoke `approval_gate.sh` and identify any approval JSONL file containing at least one record. 2. Supply a regular-expression value instead of a literal approval identifier, for example: ```bash bash scripts/approval_gate.sh --file approval.jsonl --check --id '.*' ``` 3. `grep` interprets `.*` as a wildcard and matches an unrelated record. 4. The script returns exit status zero and prints `approval_status=found`. 5. A downstream operator or automation process trusts this result and proceeds with an operation that was not actually approved. ### Impact Assessment An attacker able to control ...[truncated 492 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Parse each JSONL line with a real JSON parser instead of searching serialized JSON with `grep`. - Compare identifiers and scopes using exact string equality. - Reject malformed JSON records rather than silently treating them as searchable text. - Bind each approval to all relevant attributes, including the exact action or command digest, scope, risk, operator, approver, rollback owner, creation time, and expiration. - Reject duplicate approval IDs and define explicit revocation and expiry semantics. - If shell-only portability is mandatory, escape every regular-expression metacharacter and use fixed-string matching, although structured JSON parsing remains the preferred solution. - Add regression tests for values such as `.*`, `[a-z]`, `^`, `$`, backslashes, quotes, embedded newlines, duplicate IDs, expired approvals, and mismatched scopes. ]]>