T09 · Insecure Skill Coding Practices
Error
- Location
scripts/data_reader.py:224- Finding
SQL Injection Through Unvalidated Table Identifiers
- Content
View full analysis
List[Dict]: """ Execute a custom SQL query. Args: sql: SQL statement Returns: Query result list """ if not self.connection: if not self.connect(): return [] try: with self.connection.cursor() as cursor: cursor.execute(sql) return cursor.fetchall() except Exception as e: print(f"SQL execution failed: {e}") return [] ``` ### Technical Analysis The `table_name` argument originates from the command-line `--tables` option and is directly interpolated into SQL statements. It is not checked against a table allowlist, validated as a single SQL identifier, or safely quoted. Prepared-statement placeholders generally cannot be used for table names. Consequently, dynamic identifiers must be validated separately. The current implementation permits a crafted table argument to introduce SQL syntax into the `SELECT` statement. For example, an attacker could attempt to inject clauses or a `UNION` expression, subject to column compatibility and database-driver restrictions. The `where` argument is also concatenated directly. Although the current CLI does not expose it, this becomes another injection sink if the class is imported and called by other Skill components. Similarly, `execute ...[truncated 1799 chars]- Remediation
View remediation
str: if not IDENTIFIER_PATTERN.fullmatch(value): raise ValueError(f"Invalid SQL identifier: {value!r}") return value ``` 2. Enforce an explicit allowlist of schemas and tables approved for test-data access. Syntax validation alone does not prevent callers from selecting sensitive but valid table names. 3. Quote validated identifiers with a database-aware mechanism. Do not treat quoting as a substitute for validation and authorization. 4. Remove the `where` argument if it is not required. If filtering is needed, expose structured column/operator/value inputs, validate columns and operators, and bind values using query parameters. 5. Remove `execute_query()` unless arbitrary SQL is essential. If it must remain, restrict it to trusted internal callers and enforce read-only statements rather than accepting arbitrary text. 6. Use a dedicated read-only MySQL account that can access only approved test schemas and tables. Do not use production application credentials. 7. Add security tests covering spaces, comments, quotes, semicolons, `UNION`, qualified identifiers, and attempts to access non-allowlisted tables. ]]>