2026-09-05 · 8 min read

Python code review exercise: a search query becomes SQL

In October 2015, a teenager broke into TalkTalk, the British telecom, using a SQL injection attack against a web page that built queries from user input. The breach exposed personal details of around 157,000 customers, including bank account numbers. The ICO fined TalkTalk £400,000, which at the time was their largest penalty ever, for failing to implement what the ruling called "basic security measures." The specific basic measure in question was parameterized queries: don't put user strings inside SQL. That was 2015. This is a Python code review exercise where an AI-generated FastAPI endpoint does exactly the same thing, in 2026, in four lines of code that look completely normal until you read what the f-string actually does. It is a worked example with the full solution: the trigger, the exploit, and the one-line structural fix. If you are practicing Python code review, learning to catch security bugs in AI-written code, or building a habit of spotting injection flaws before they ship, this is the exercise.

What the PR is supposed to do

The task was simple: add a user search endpoint to an existing FastAPI app. The requirements, word for word:

1. Add GET /users/search?q= returning users whose name contains q (literal substring, case-insensitive).
2. q comes straight from the query string (untrusted).
3. Use the project's Encode databases.Database helper: db.fetch_all(query, values) with :name bind parameters.

The context says the users table holds emails and password hashes. The endpoint is public, no auth required. That combination should already have your attention: a public endpoint querying a table full of credentials.

The diff

Ten lines. Clean imports, a router, one endpoint, one query. It would return the right results for every honest search you tried. Type "ann" and you get Ann. Type "bob" and you get Bob. Nothing looks wrong. The tests would pass. The demo would work.

Try it yourself first

Walkthrough starts below. If you haven't reviewed the diff yet and want to try it yourself, stop here.

Four lines, one of which is a door

The entire bug lives on line 9:

sql = f"SELECT id, name FROM users WHERE name LIKE '%{q}%'"

This is an f-string. Whatever the caller puts in the q query parameter becomes part of the SQL statement, character for character, with no escaping, no quoting, no parameterization. The q variable is not data being passed to a query. It is the query. The single quotes around %{q}% are part of the SQL string literal, and the caller can close them whenever they want.

Try q = ' OR '1'='1. The SQL becomes:

SELECT id, name FROM users WHERE name LIKE '%' OR '1'='1%'

The WHERE clause now matches every row, because '1'='1' is always true (the trailing %' is a string literal that evaluates to truthy in the OR, or you adjust the payload slightly to clean up the syntax). The attacker just dumped every user in the table through a search box.

But dumping names is the gentle version. The table holds emails and password hashes, and this endpoint only selects id, name. A UNION SELECT fixes that:

q = ' UNION SELECT email, password_hash FROM users --

Now the response contains every email and hashed password in the database, formatted as if they were search results. The -- comments out the trailing %' so the syntax stays valid. The endpoint is public, so no authentication is needed to run this. Anyone with a browser and five minutes of SQL knowledge can extract the full credentials table.

Whether stacked statements like '; DROP TABLE users; -- work depends on the database driver. The databases library with its default async backends typically does not allow multiple statements in a single call, so a DROP TABLE would likely fail. But the data exfiltration via UNION SELECT works on every SQL backend, and that is already catastrophic. Do not promise DROP TABLE as the headline exploit on every driver, but do not pretend the injection is theoretical either: the UNION path is unconditional.

Which requirement did the code break?

Requirement 3: "Use the project's Encode databases.Database helper: db.fetch_all(query, values) with :name bind parameters." The code calls db.fetch_all(sql) with one argument. There is no values dict, no :name placeholder. The requirement explicitly told the author how to pass user input safely, and the code ignored it and used string interpolation instead.

Requirement 2 also matters: "q comes straight from the query string (untrusted)." The requirement labeled the input as untrusted. The code treats it as trusted. That is not a subtle misunderstanding. It is the spec saying "this is a gun" and the code pointing it at itself.

The comment you would actually leave

Verdict: request changes. This is a blocker. The endpoint cannot ship with user input inside an f-string that becomes SQL. Everything else about the code is fine in structure, but structure does not matter when the foundation is an injection vulnerability against a public endpoint serving a credentials table.

The fix

Move q out of the SQL string and into a bind parameter:

@router.get("/users/search")
async def search_users(q: str):
    escaped = q.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
    sql = "SELECT id, name FROM users WHERE name LIKE :pattern ESCAPE '\\'"
    return await db.fetch_all(sql, {"pattern": f"%{escaped}%"})

Two things changed. First, q is now a bind parameter (:pattern), passed in the values dict. The database driver treats it as data, never as SQL. No amount of single quotes or UNION keywords in the input can break out of the parameter boundary, because the driver sends the query structure and the parameter value separately to the database engine. This is the fix for the injection.

Second, the LIKE metacharacters % and _ are escaped before binding. This is a separate concern from the injection. The requirement says "literal substring," which means if a user searches for the literal character %, they should get rows containing a percent sign, not every row in the table (since % is a wildcard in LIKE). Similarly, _ matches any single character in LIKE, so a search for _ without escaping would match every row whose name is at least one character long. The ESCAPE '\' clause tells the database which character introduces an escaped metacharacter.

One thing the fix does not address: the requirement says "case-insensitive," but LIKE is case-sensitive on most databases (PostgreSQL, SQLite in some configurations). A production fix would use ILIKE on PostgreSQL or wrap both sides in LOWER(). The challenge focuses on the injection, but in a real review you would flag the case-sensitivity gap as a secondary finding.

The test that distinguishes broken from fixed

The simplest verification: send a request with a single quote in the query and observe what happens.

# Against the broken code:
GET /users/search?q=' OR '1'='1
# Returns: every user in the table (SQL injection succeeds)

# Against the fixed code:
GET /users/search?q=' OR '1'='1
# Returns: empty list (no user's name literally contains that string)

For the LIKE metacharacter escaping:

# Against broken code (even if injection is fixed but % is not escaped):
GET /users/search?q=%
# Returns: every user (% is a wildcard matching everything)

# Against fixed code:
GET /users/search?q=%
# Returns: only users whose name literally contains a percent sign

And for the UNION exfiltration path specifically:

# Against the broken code:
GET /users/search?q=' UNION SELECT email, password_hash FROM users --
# Returns: every email and password hash in the response body

# Against the fixed code:
GET /users/search?q=' UNION SELECT email, password_hash FROM users --
# Returns: empty list (the entire string is treated as a literal search term)

If you wanted to write this as a pytest, you would insert two users, send q=', and assert an empty list. Then send q=ann and assert it returns Ann. The broken code raises a database syntax error or returns wrong results on the first query; the fixed code returns an empty list cleanly.

The reflex worth keeping

Every time you see user input travel from an HTTP parameter into a string that gets interpreted by another system, whether that system is a SQL database, an HTML renderer, a shell, or a CSV parser, ask one question: is the input treated as data or as code? If it is concatenated, interpolated, or formatted into the command string, it is code, and the caller controls it. Bind parameters, template escaping, and shell quoting all exist to enforce the same boundary: user input is data, never structure.

AI assistants generate the f-string form of SQL queries constantly, because f"SELECT ... WHERE name LIKE '%{q}%'" is the most natural-looking way to "put q into the query" if you are completing tokens based on what reads well. It reads well because it does exactly what it says. The problem is that what it says is "paste the user's input into the SQL," and that is the definition of injection. The code is honest about what it does. It just does something that should never be done.

The review habit is not "memorize SQL injection." The review habit is: when data crosses a format boundary, check whether it crosses as data or as structure. That single question catches SQL injection, XSS, CSV injection, command injection, and LDAP injection, because they are all the same bug wearing different hats.