apache / apache/infrastructure-asfpy
SQL Identifier Injection in SQLite Database Wrapper
- Dominant language
- Python
- Stars
- 4
- Forks
- 6
- PR merge metrics
- No merged PRs in 30d
Description
## Issue: FINDING-071 - SQL Identifier Injection in SQLite Database Wrapper
**Labels:** bug, security, priority:medium, asvs-level:L1
**ASVS Level(s):** [L1]
**Description:**
### Summary
The DB class in `asfpy/sqlite.py` constructs SQL statements by directly interpolating table names and dictionary keys (representing column names) into SQL strings via f-strings and %s formatting. While values are correctly parameterized using ? placeholders, identifiers (table names, column names, LIMIT clauses) receive no sanitization, escaping, or allowlist validation. This creates SQL injection vulnerability if table/column names or limit values are derived from user input. No active exploitation path identified in ATR application as it uses SQLAlchemy/SQLModel exclusively.
### Details
**Affected Files and Lines:**
- `asfpy/sqlite.py:66` - delete() with identifier interpolation
- `asfpy/sqlite.py:78` - update() with identifier interpolation
- `asfpy/sqlite.py:94` - insert() with identifier interpolation
- `asfpy/sqlite.py:106` - upsert() with identifier interpolation
- `asfpy/sqlite.py:135` - fetch() with identifier interpolation
Table and column names are interpolated directly into SQL without validation or quoting, creating injection risk.
### Recommended Remediation
Add identifier validation using regex pattern and quote identifiers with double-quotes:
```python
import re
_IDENTIFIER_PATTERN = re.compile(r'^[a-zA-Z_][a-zA-Z0-9_]*$')
def _validate_identifier(identifier: str) -> str:
"""Validate and quote SQL identifier."""
if not _IDENTIFIER_PATTERN.match(identifier):
raise ValueError(f"Invalid SQL identifier: {identifier}")
return f'"{identifier}"'
# Apply in all methods
def delete(self, table: str, where: dict) -> None:
"""Delete rows with validated identifiers."""
table = _validate_identifier(table)
columns = [_validate_identifier(k) for k in where.keys()]
# ... rest of function
```
Apply `_validate_identifier()` function to all table names, column names in `delete()`, `update()`, `insert()`, `upsert()`, and `fetch()` methods. Parameterize the limit value in `fetch()` method. Fix inconsistent column quoting to use double-quotes throughout.
### Acceptance Criteria
- [ ] Identifier validation function added
- [ ] All table names validated
- [ ] All column names validated
- [ ] Identifiers quoted with double-quotes
- [ ] Limit value parameterized
- [ ] Unit test verifying the fix
### References
- Source reports: L1:1.2.4.md
- Related findings: None
- ASVS sections: 1.2.4
### Priority
Medium
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.