How SQL Injection happens in Python SQLite utilities and how to fix it

How SQL Injection Happens in Python SQLite Utilities and How to Fix It

Vulnerability at a Glance

Quick Answer

What is this vulnerability and how do you fix it?

The _get_or_create() function in scripts/db_utils.py used Python f-strings to interpolate the table and col arguments directly into SQL query strings. While SQLite's ? placeholder correctly protected the value being inserted or searched, it cannot protect table or column names — those identifiers were wide open to injection. The fix replaces all dynamic SQL construction with a pre-built dictionary (_SQL_SELECT / _SQL_INSERT) that maps only approved (table, column) pairs to static, parameterized query strings. Any unlisted combination raises a ValueError, making the attack surface effectively zero.

Introduction

The scripts/db_utils.py file is the backbone of this application's database layer — it initialises the schema, stores prompts, model names, and error messages, and provides the shared _get_or_create helper that every other database function depends on. That helper, however, contained a structural flaw that turned a routine database utility into a latent SQL injection vector.

At line 66, _get_or_create accepted two free-form string parameters — table and col — and embedded them directly into SQL using an f-string:

# BEFORE — vulnerable code at db_utils.py:66
row = conn.execute(f"SELECT id FROM {table} WHERE {col} = ?", (value,)).fetchone()

The ? placeholder correctly protected value, but table and col were interpolated with no validation whatsoever. Any future caller that passed user-influenced strings to this function would hand an attacker direct control over the structure of the SQL statement itself.

The Vulnerability Explained

Why f-strings in SQL are dangerous — even with ? placeholders

SQLite's parameterised query interface (the ? placeholder) is excellent at preventing injection through values — the data being stored or compared. But SQL identifiers — table names, column names, schema names — cannot be parameterised through the standard placeholder mechanism. The database driver treats them as structural parts of the query, not as data.

This means the only safe options for dynamic identifiers are:

1. Validate against a hardcoded allowlist before using them.
2. Never allow them to be dynamic at all.

The original _get_or_create did neither. Here is the full vulnerable function:

# BEFORE — scripts/db_utils.py (original)
def _get_or_create(conn: sqlite3.Connection, table: str, col: str, value: Any) -> int | None:
    if not value:
        return None
    row = conn.execute(f"SELECT id FROM {table} WHERE {col} = ?", (value,)).fetchone()
    if row:
        return row[0]
    cur = conn.execute(f"INSERT INTO {table} ({col}) VALUES (?)", (value,))
    return cur.lastrowid

Both the SELECT and the INSERT statements are built with {table} and {col} interpolated directly. There is no check that table is a real table, that col is a real column, or that either string is free of SQL metacharacters.

The exploitation scenario

Suppose a future developer adds an API endpoint that lets users specify a "category" for their prompt, and that category is passed down to _get_or_create. An attacker could supply:

table = "prompts WHERE 1=1; DROP TABLE models;--"

The resulting query would become:

SELECT id FROM prompts WHERE 1=1; DROP TABLE models;-- WHERE text = ?

Depending on the SQLite configuration and Python driver version, this could:

• Exfiltrate data from tables the caller never intended to expose.
• Destroy data by injecting DROP TABLE or DELETE statements.
• Bypass application logic by injecting WHERE 1=1 conditions that always return a result.

The PR notes that the same risky pattern appears at lines 124 and 125 of the same file, meaning the attack surface was not limited to a single call site.

Why "current callers use hardcoded values" is not a sufficient defence

This is a common rationalisation that leads to vulnerabilities surviving code review. The function's signature accepts arbitrary strings. The moment a new developer calls _get_or_create(conn, user_input_table, user_input_col, value) — perhaps while adding a feature under time pressure — the application becomes immediately exploitable. Security must be enforced at the function boundary, not assumed from caller discipline.

The Fix

The fix introduces two compile-time dictionaries that map every approved (table, column) pair to a fully static, pre-written SQL string. Dynamic construction is eliminated entirely.

Before and After

Before (vulnerable):

def _get_or_create(conn: sqlite3.Connection, table: str, col: str, value: Any) -> int | None:
    if not value:
        return None
    row = conn.execute(f"SELECT id FROM {table} WHERE {col} = ?", (value,)).fetchone()
    if row:
        return row[0]
    cur = conn.execute(f"INSERT INTO {table} ({col}) VALUES (?)", (value,))
    return cur.lastrowid

After (fixed):

_SQL_SELECT: dict[tuple[str, str], str] = {
    ("prompts", "text"): "SELECT id FROM prompts WHERE text = ?",
    ("models", "name"):  "SELECT id FROM models WHERE name = ?",
    ("errors", "text"):  "SELECT id FROM errors WHERE text = ?",
}
_SQL_INSERT: dict[tuple[str, str], str] = {
    ("prompts", "text"): "INSERT INTO prompts (text) VALUES (?)",
    ("models", "name"):  "INSERT INTO models (name) VALUES (?)",
    ("errors", "text"):  "INSERT INTO errors (text) VALUES (?)",
}

def _get_or_create(conn: sqlite3.Connection, table: str, col: str, value: Any) -> int | None:
    if not value:
        return None
    key = (table, col)
    if key not in _SQL_SELECT:
        raise ValueError(f"Disallowed table/column combination: {table!r}, {col!r}")
    row = conn.execute(_SQL_SELECT[key], (value,)).fetchone()
    if row:
        return row[0]
    cur = conn.execute(_SQL_INSERT[key], (value,))
    return cur.lastrowid

Why this fix works

1. Zero dynamic SQL construction. Every query string in _SQL_SELECT and _SQL_INSERT is a string literal written by a developer, not assembled at runtime. There is no path through which attacker-controlled input can alter the structure of a SQL statement.

2. Explicit allowlist with hard rejection. The if key not in _SQL_SELECT guard means that any (table, col) combination not explicitly approved raises a ValueError immediately. This turns a silent, exploitable path into a loud, visible error that will surface during development and testing — not in production.

3. The ? placeholder still protects values. The value parameter continues to be passed as a bound parameter, so the fix does not regress the existing protection for data inputs.

4. Future additions require a deliberate decision. Adding a new table/column combination to the allowlist requires a developer to consciously write a new entry in _SQL_SELECT and _SQL_INSERT. This creates a natural review checkpoint that the old f-string approach completely lacked.

Key Takeaways

• f-strings and SQL identifiers don't mix. The ? placeholder in Python's sqlite3 module protects values, not table or column names. The _get_or_create function's use of f"SELECT id FROM {table}" was unsafe regardless of current caller behaviour.
• A function's signature defines its attack surface. Because _get_or_create accepted arbitrary table and col strings, any future caller — not just today's callers — could introduce injection. Security must be enforced at the function boundary.
• Allowlists beat blocklists for SQL identifiers. You cannot reliably sanitise all possible SQL injection payloads from an identifier string. Mapping only approved (table, col) pairs to static query strings is provably safe.
• The fix is also a design improvement. The _SQL_SELECT and _SQL_INSERT dictionaries make every supported table/column combination explicit and auditable at a glance — something the original dynamic approach never provided.
• Lines 124–125 needed review too. The scanner flagged the primary vulnerable line, but the PR note about additional occurrences at lines 124 and 125 is a reminder that shared utility functions often have multiple call sites, all of which need attention.

How Orbis AppSec Detected This

• Source: The table and col parameters of _get_or_create(conn, table, col, value) in scripts/db_utils.py — both accept arbitrary string input with no validation.
• Sink: conn.execute(f"SELECT id FROM {table} WHERE {col} = ?", ...) and conn.execute(f"INSERT INTO {table} ({col}) VALUES (?)", ...) at line 66 (and related patterns at lines 124–125).
• Missing control: No allowlist, no identifier quoting, and no type or value validation on the table and col parameters before they were interpolated into the SQL string.
• CWE: CWE-89 — Improper Neutralization of Special Elements used in an SQL Command.
• Fix: Replaced both f-string queries with lookups into _SQL_SELECT and _SQL_INSERT dictionaries that map only approved (table, column) pairs to fully static, parameterized SQL strings, and added a ValueError guard for any unlisted combination.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

The _get_or_create vulnerability in scripts/db_utils.py is a textbook example of how a well-intentioned utility function can become a security liability over time. The original code worked correctly with its current callers — but its open-ended signature meant that a single future change could have introduced a serious SQL injection flaw into the application's core database layer.

The fix is elegant precisely because it doesn't just patch the immediate problem: it restructures the function so that SQL injection through identifier interpolation is architecturally impossible. The allowlist dictionaries _SQL_SELECT and _SQL_INSERT serve as living documentation of the function's intended scope, and the ValueError guard ensures that any attempt to use the function outside that scope fails loudly and immediately.

For developers working with raw SQL in Python — whether with sqlite3, psycopg2, or any other driver — the lesson is clear: parameterise your values, and allowlist your identifiers. There is no safe middle ground.