SQL injection: how to spot it in a code review

SQL injection: how to spot it in a code review

TL;DR

Every query that mixes untrusted input into SQL text is a finding until proven parameterized. In review, hunt for string concatenation, f-strings, and percent-formatting near SQL keywords, then confirm each query passes values as separate parameters. If the values ride along inside the SQL string, flag it.

SQL injection: how to spot it in a code review

Use this when

• You are reviewing a PR that adds or changes database queries
• You are auditing an older codebase you did not write
• A teammate asks "is this query safe" and you need a repeatable answer
• You are building a security checklist for your team's review process
• An agent is doing automated first-pass review before a human looks

Not for this skill when

• The issue is cross-site scripting, not SQL (different sinks, different fixes)
• You are dealing with NoSQL query operators (the patterns rhyme but the checks differ)
• The database is already suspected compromised (that is incident response, not review)
• You need to tune query performance (index talk belongs elsewhere)

Steps

1. List every place the PR builds SQL

Run a search for SQL keywords and raw-execution calls across the changed files.

grep -rn "SELECT\|INSERT\|UPDATE\|DELETE\|\.raw(\|\.execute(" --include="*.py" --include="*.js" --include="*.ts" .

Expected: a short list of files and line numbers. If the list is empty, there is no SQL surface in this PR and you are done.

2. Flag concatenation and interpolation

Open each hit and look for the query text being assembled: plus operators, f-strings or template literals, percent formatting, or .format() calls that include variables.

Vulnerable shape (flag this):
query = "SELECT * FROM accounts WHERE name = '" + user_input + "'"
Safe shape (values travel separately):
cursor.execute("SELECT * FROM accounts WHERE name = %s", (user_input,))

Expected: you can point at each query and say which of the two shapes it matches. Anything matching the first shape is a finding, even if the input "comes from our own frontend".

3. Check ORM raw-query escapes

ORMs are safe until someone drops to raw SQL. Search for the escape hatches: .raw(, execute(, text(, query(, sequelize.query, knex .raw.

grep -rn "\.raw(\|execute(\|sequelize\.query\|knex\.raw" --include="*.py" --include="*.js" --include="*.ts" .

Expected: for each hit, the values are passed as a separate bindings argument, never interpolated into the SQL string. A raw call with an f-string inside is still injection.

4. Verify the parameter style matches the driver

Each driver has its own placeholder style (%s, ?, $1, or named params like :name). Confirm the code uses the driver's style and passes a tuple, list, or dict of values as the second argument.

Expected: no % formatting or .format() applied to the SQL string itself. Formatting the values is fine; formatting the query text is the bug.

5. Look for second-order injection

Check whether values read back out of the database later get concatenated into another query. Stored data is still untrusted data.

Expected: every query that consumes stored values is parameterized too, or the review notes the gap explicitly.

6. Ask for a regression test with a quote character

The cheapest proof is a test that submits a name containing a single quote and asserts the query still works and returns the right row.

pytest tests/test_accounts.py -k "quote" -v

Expected: the new test passes against the parameterized query. If the author pushes back, the failing-then-passing test is your evidence.

Variant: spotting SQL injection in Node.js reviews

In JS/TS codebases the same concatenation shows up as template literals: ` SELECT ... WHERE id = ${req.params.id} . The safe shape is db.query("SELECT ... WHERE id = ?", [req.params.id]) or $1 style for postgres. Grep for backtick SQL and ${` inside query calls.

Variant: ORM raw queries are the usual hiding spot

Teams that "use an ORM so we are safe" still get bitten in migrations, reporting queries, and search filters where someone wrote raw SQL for flexibility. Treat every .raw( as guilty until the bindings argument is confirmed.

Variant: LIKE clauses and ORDER BY need extra care

Wildcards in LIKE come from the value, which is fine, but a user-controlled column name in ORDER BY cannot be parameterized. The fix there is an allowlist of column names, not a placeholder. Flag any ORDER BY ${sort} pattern and ask for the allowlist.

Why this happens

SQL treats code and data as one string, so the database cannot tell which parts the developer wrote and which parts the user supplied. Concatenation erases that boundary. Parameterized queries restore it by sending the query shape and the values through separate channels, which is why "just escape the quotes" keeps failing: escaping is a guess about every edge case, parameters are a structural guarantee.

Edge cases and pitfalls

• Error messages that echo SQL text leak schema details; flag verbose DB errors in the PR too.
• Query logging that prints full SQL with values can land PII in logs; suggest logging the parameterized shape instead.
• Table and column names cannot be parameterized in most drivers; allowlist them or reject the pattern.
• Batch import scripts and admin tooling get reviewed less but run with more privilege; do not skip them.

Provenance

Resolved from the public thread: https://vectle.com/posts/pst_ZDSIMHE5AZxSQLKjiYehhg