T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/db.sh:400
- Finding
- Unsanitized Identifiers Allow Injection into Generated SQL<![CDATA[ ## Vulnerability Details **File Location**: `scripts/db.sh:400-437` **Additional Affected Locations**: `scripts/db.sh:158-162`, `scripts/db.sh:483`, `scripts/db.sh:583-591` **Vulnerability Type**: SQL injection through unsafe SQL generation **Risk Level**: Medium ### Vulnerable Code ```bash # Generate index name if not provided if [[ -z "$INDEX_NAME" ]]; then local sanitized sanitized=$(echo "$field_list" | tr ',' '_' | tr -d ' ') INDEX_NAME="idx_${TABLE}_${sanitized}" fi echo "-- ============================================================" echo "-- Index: ${INDEX_NAME}" echo "-- Table: ${TABLE}" echo "-- Fields: ${field_list}" echo "-- Generated: ${DATE}" echo "-- ============================================================" echo "" # Format fields for SQL local formatted_fields="" IFS=',' read -ra idx_fields <<< "$field_list" for i in "${!idx_fields[@]}"; do local f f=$(echo "${idx_fields[$i]}" | xargs) if [[ $i -gt 0 ]]; then formatted_fields+=", " fi if [[ "$ENGINE" == "postgres" ]]; then formatted_fields+="\"${f}\"" else formatted_fields+="\`${f}\`" fi done if [[ "$ENGINE" == "postgres" ]]; then echo "CREATE ${UNIQUE}INDEX ${INDEX_NAME}" echo " ON \"${TABLE}\" (${formatted_fields});" else echo "CREATE ${UNIQUE}INDEX ${INDEX_NAME}" echo " ON \`${TABLE}\` (${formatted_fields});" fi ``` Other affected SQL generation includes: ```bash if [[ "$ENGINE" == "postgres" ]]; then echo "CREATE TABLE IF NOT EXISTS \"${TABLE}\" (" else echo "CREATE TABLE IF NOT EXISTS \`${TABLE}\` (" fi ``` ```bash echo "INSERT INTO \`${TABLE}\` (${names_str}) VALUES" ``` ```bash case "$ACTION" in add) echo "ALTER TABLE \`${TABLE}\` ADD COLUMN \`${fname}\` ${sql_type};" ;; drop) echo "ALTER TABLE \`${TABLE}\` DROP COLUMN \`${fname}\`;" ;; modify) echo "ALTER TABLE \`${TABLE}\` MODIFY COLUMN \`${fname}\` ${sql_type};" ;; ``` ### Technical Analysis The script incorporates user-control ...[truncated 2986 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate every user-controlled SQL identifier before generating output. A suitable conservative grammar is: ```bash validate_identifier() { local identifier="$1" if [[ ! "$identifier" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then printf 'Error: invalid SQL identifier: %q\n' "$identifier" >&2 exit 1 fi } ``` 2. Apply validation independently to: - `TABLE` - Every field name parsed from `FIELDS` - The field name parsed from `FIELD` - Every index field - User-supplied `INDEX_NAME` - Automatically generated index names 3. Reject rather than normalize identifiers containing quotes, backticks, semicolons, whitespace, SQL comments, newlines, control characters, or other punctuation. 4. Quote validated identifiers consistently for the selected database engine. Validation should remain mandatory even when quoting is used. 5. Add an engine-aware identifier helper, for example: ```bash quote_identifier() { local identifier="$1" validate_identifier "$identifier" if [[ "$ENGINE" == "postgres" ]]; then printf '"%s"' "$identifier" else printf '`%s`' "$identifier" fi } ``` 6. Do not emit custom index names as unquoted raw text. Validate and pass them through the same quoting helper used for other identifiers. 7. Add regression tests using hostile inputs containing: - Backticks and double quotes - Semicolons - `--` and `/* ... */` comments - Newlines and control characters - Spaces and shell metacharacters - Database reserved words 8. Clearly document that generated SQL must be reviewed before execution and should be applied using a least-privileged database account. This is defense in depth and must not replace validation. ]]>