Unvalidated Input on a Public Endpoint
Prompt
This PR adds a search endpoint to our public API — anyone on the internet can
hit it with a name query parameter. It's framed as a small feature add, the
tests pass, and the author wants a quick approve. Review it.
app.get("/api/users/search", async (req, res) => {
const name = req.query.name;
const sql = "SELECT id, email FROM users WHERE name = '" + name + "'";
const results = await db.query(sql);
res.json(results);
});
How this round runs
Rank what you find by severity. The author is asking for a fast approve — say whether you'd give it and what the most serious issue is, even though nothing in the PR description mentions security.
Model answer
Hard block. The PR is framed as a small feature, but the headline finding is a critical security hole — and I'd flag it unprompted, because severity, not the PR description, sets the priority.
1 (highest — SQL injection, unauthenticated, on a public endpoint). name
comes straight from the query string and is concatenated into the SQL string. An
attacker controls part of the query. Passing
?name=' OR '1'='1 turns the WHERE into a tautology and dumps every user's id
and email; ?name=' UNION SELECT ...-- exfiltrates arbitrary columns/tables;
on some drivers stacked statements enable writes or drops. This is reachable by
anyone on the internet with no auth — about the worst severity class there is.
The passing tests prove nothing here: they exercise benign inputs, and injection
lives entirely in the inputs the tests don't send.
Root cause: untrusted input is interpolated into a query instead of being passed as a bound parameter, so data and code share the same channel. The fix is a parameterized query — the value can never be parsed as SQL:
app.get("/api/users/search", async (req, res) => {
const name = req.query.name;
if (typeof name !== "string" || name.length === 0 || name.length > 100) {
return res.status(400).json({ error: "invalid name" });
}
const results = await db.query(
"SELECT id, email FROM users WHERE name = $1",
[name]
);
res.json(results);
});
2 (security — data exposure). Even fixed, the endpoint returns email for
arbitrary users to an unauthenticated caller. That's a PII-disclosure / user-
enumeration problem independent of injection — I'd question whether this should
require auth and whether email belongs in the response at all.
3 (resource / abuse). No input validation, no length cap, no pagination/limit,
no rate limiting on a public endpoint — an open invitation for scraping and
heavy unbounded queries. The validation in the fix above is the floor; rate
limiting and a LIMIT are the next asks.
Note the ordering: injection first (catastrophic, remote, unauthenticated), then data exposure, then abuse-hardening. I'd state the block is about the security findings, not style — variable names and formatting are fine and I won't hold the PR on them. No fast approve; this one gets the slow, careful kind.
- Flagged SQL injection unprompted and ranked it #1 despite the PR being framed as a harmless feature
- Showed a concrete exploit payload (' OR '1'='1 / UNION) rather than asserting 'injection-prone'
- Named the root cause as mixing data and code in one channel, fixed with parameterized queries — not string escaping
- Surfaced the secondary security issue (unauthenticated PII/email exposure) separate from the injection
- Refused the fast approve and added input validation + rate-limit/limit as resource-abuse hardening
- Isn't escaping the quotes enough? → No — escaping is fragile and driver-specific; bound parameters separate data from code by construction.
- It's just a search box, who'd attack it? → It's a public, unauthenticated endpoint; assume hostile input. Severity comes from reachability, not intent.
- What beyond the injection would you require? → Auth or a reason it's public, drop or gate email, input validation, a LIMIT, and rate limiting.
// write your answer here