# Secure code review interview questions for security engineers, worked Source: https://www.techinterview.org/post/3233477869/secure-code-review-interview/ Updated: 2026-11-05 ยท techinterview.org A secure code review round usually hands you one short file; this one is 80 lines. You get 45 to 60 minutes to say what you would block from shipping. You pass by triaging on exploitability: name the SQL injection and the path traversal first, because either one alone exposes the whole database, say the impact of each in a sentence, then work down. Below is one vulnerable Flask invoice service with eight bugs ranked by severity, each with how it is exploited and the fix, plus a few more that sit below the line. The exploitable findings were run against a local copy on Flask 3.1; the SSRF example used a local stand-in for the cloud metadata endpoint, not a real one. Most secure code review rounds look the same once you have sat a few. The interviewer pastes a short service into a shared editor, tells you it ships next week, and asks you to review it the way you would a real pull request. The round goes by a few names. Amazon lists "Secure code review" among its technical competencies for security engineers ([Amazon's security engineer prep page](https://amazon.jobs/content/en/how-we-hire/security-engineer-interview-prep), where the loop is five 60-minute interviews). A 2024 candidate report from a Meta new-grad ProdSec onsite describes a round called "Security Code Review (Web In Domain)" on "a vulnerable server side codebase in PHP" that the candidate had never used ([candidate report](https://leetcode.com/discuss/interview-experience/4965161/)). The job post rarely tells you which language. So the exercise below is the generic version: a small Flask invoice API with bugs seeded across the severity range. ``` 1 import os 2 import sqlite3 3 import traceback 4 import requests 5 from flask import Flask, abort, g, jsonify, request, send_file, session 6 from werkzeug.security import check_password_hash 7 8 app = Flask(__name__) 9 app.secret_key = "inv-prod-7f3a9c2e41" 10 ADMIN_TOKEN = os.environ["ADMIN_TOKEN"] 11 UPLOAD_DIR = "/srv/invoices/uploads" 12 DB_PATH = "/srv/invoices/app.db" 13 14 def db(): 15 if "db" not in g: 16 g.db = sqlite3.connect(DB_PATH) 17 g.db.row_factory = sqlite3.Row 18 return g.db 19 20 def one(sql, *args): 21 return db().execute(sql, args).fetchone() 22 23 def current_user(): 24 uid = session.get("user_id") 25 if uid is None: 26 abort(401) 27 return one("SELECT * FROM users WHERE id = ?", uid) 28 29 @app.post("/login") 30 def login(): 31 body = request.get_json() 32 user = one("SELECT * FROM users WHERE email = ?", body["email"]) 33 pw = body["password"] 34 if not user or not check_password_hash(user["pw_hash"], pw): 35 return jsonify(error="invalid credentials"), 401 36 session["user_id"] = user["id"] 37 return jsonify(ok=True) 38 39 @app.get("/invoices/") 40 def get_invoice(invoice_id): 41 current_user() 42 row = one("SELECT * FROM invoices WHERE id = ?", invoice_id) 43 if row is None: 44 abort(404) 45 return jsonify(dict(row)) 46 47 @app.get("/invoices/search") 48 def search_invoices(): 49 user = current_user() 50 q = request.args.get("q", "") 51 sql = ("SELECT id, customer, total FROM invoices" 52 f" WHERE org_id = {user['org_id']}" 53 f" AND customer LIKE '%{q}%'") 54 return jsonify([dict(r) for r in db().execute(sql).fetchall()]) 55 56 @app.get("/attachments") 57 def download_attachment(): 58 current_user() 59 return send_file(os.path.join(UPLOAD_DIR, request.args["name"])) 60 61 @app.post("/invoices//logo") 62 def fetch_logo(invoice_id): 63 current_user() 64 resp = requests.get(request.get_json()["url"], timeout=5) 65 name = f"logo-{invoice_id}.png" 66 with open(os.path.join(UPLOAD_DIR, name), "wb") as f: 67 f.write(resp.content) 68 return jsonify(attachment=name, bytes=len(resp.content)) 69 70 @app.post("/admin/purge") 71 def purge(): 72 if request.headers.get("X-Admin-Token") != ADMIN_TOKEN: 73 abort(403) 74 db().execute("DELETE FROM invoices WHERE deleted = 1") 75 db().commit() 76 return jsonify(ok=True) 77 78 @app.errorhandler(500) 79 def on_error(e): 80 return jsonify(error=traceback.format_exc()), 500 ``` The numbers down the left are part of the listing, the way a shared editor shows them; the code itself is a single file. Spend your first two minutes reading top to bottom without saying anything, and build a mental model of two things only: how a request proves who it is, and where user input reaches a database, the filesystem, or the network. Everything that matters in this file sits at one of those crossings. If you want to work it like the real round, stop here and give yourself 20 minutes before reading on. ## How a secure code review interview is graded The thing being measured is triage, not completeness. An interviewer who has run this round many times is listening for whether the first bug out of your mouth is the one that loses the whole database, or a missing timeout. Lead with impact, in two or three short sentences. The search endpoint is injectable, so a logged-in user dumps every tenant's invoices and the password hashes. The attachments endpoint hands back any file the server can read, including the database itself. Block the release on those two, then go deep on them before anything else. Say out loud what you would confirm with tooling rather than pretending to be a parser. You would run this through `bandit` and a Semgrep ruleset to catch the string-built SQL and the hard-coded secret, grep the git history for that key in case it is already committed, and point a secrets scanner and a dependency scanner at the repo. That tells the interviewer you know where a human review stops and where static analysis takes over. It also buys cover: you are claiming to have found everything that matters, and to know how you would catch the rest. The gap between a pass and a strong pass is the second sentence on each finding. A pass says "line 54 is SQL injection." A strong pass says "line 54 is SQL injection, one query reads every org's rows and the users table, and the fix is a parameterized query, not escaping." The strong candidate also notices an inconsistency worth catching: the search endpoint remembered to scope by `org_id` and the single-invoice endpoint forgot, which is exactly how real access-control bugs ship. This round is a sibling of the [threat modeling interview](https://www.techinterview.org/post/3233477868/threat-modeling-interview/) and sits in the same loop. The full set of [security engineer interview guides](https://www.techinterview.org/security-engineer-interview-guides/) shows how the rounds fit together, and the [security engineer loop](https://www.techinterview.org/post/3233476398/security-engineer-loop-judgment-over-trivia/) post covers what each interviewer is scoring. ## The eight findings, ranked Severity here is blast radius from the position of an ordinary logged-in user of one tenant, weighed by how much the attacker needs first. CWE IDs and OWASP categories are from the Top 10:2025 at top10.owasp.org, where SSRF is now folded into A01 Broken Access Control and A10 Mishandling of Exceptional Conditions is a new category. These are the eight worth ranking inside the time box; a few more sit below the line, named after the fixes. | Severity | Finding and location (sample Flask invoice API) | Impact if shipped | OWASP 2025 / CWE | One-line fix | | --- | --- | --- | --- | --- | | Critical | SQL injection in search_invoices (lines 51โ€“54), query built by f-string | Read every tenant's invoices and the users table, including password hashes, with one UNION | A05:2025 Injection / CWE-89 | Parameterized query with ? placeholders | | Critical | Path traversal in download_attachment (line 59), filename taken from the request | name=../app.db returns the whole SQLite database; an absolute path returns any file the process can read, including /proc/self/environ with the admin token in it | A01:2025 Broken Access Control / CWE-22 | send_from_directory, which rejects traversal | | High; Critical where IMDSv1 is on | SSRF in fetch_logo (line 64), server fetches a user-supplied URL | On AWS with IMDSv1 enabled, aim it at the instance metadata role-credentials path; the server fetches the temporary keys, stores them in the attachments directory, and they are readable via /attachments. IMDSv2's token and the GCP and Azure header requirements stop a bare GET | A01:2025 Broken Access Control / CWE-918 | Validate scheme and every resolved IP, block non-global addresses, pin the connection, cap the body | | High | Hard-coded session signing key (line 9), secret_key literal in source | Anyone who reads the source, or extracts it through the SQL injection or path traversal, forges a session cookie for any user | A04:2025 Cryptographic Failures / CWE-321 | Load from the environment or a secrets manager; rotate the leaked value | | High | Broken object-level access control in get_invoice (line 42), lookup by id with no tenant check | Any logged-in user reads any invoice by walking the integer ids | A01:2025 Broken Access Control / CWE-639 | Add AND org_id = ? to the lookup | | Medium | Verbose error handler (lines 78โ€“80) returns the full traceback | Leaks filesystem paths, the framework stack frames, and the database's own error text to any client that can trigger a 500 | A10:2025 Mishandling of Exceptional Conditions / CWE-209 | Log the traceback server-side, return a generic message | | Medium | No throttling on login (lines 29โ€“37) | Unlimited password guesses per account; credential stuffing runs at full speed | A07:2025 Authentication Failures / CWE-307 | Per-account and per-IP rate limiting with lockout or backoff | | Low | Non-constant-time token compare in purge (line 72), != on the admin token | A timing side channel in theory; hard to exploit remotely, but the wrong primitive for a secret | No Top 10:2025 category / CWE-208 | hmac.compare_digest | ## The top three, with fixes The SQL injection is the one to call first because it is the widest. Lines 51 to 54 build the query by interpolating both `org_id` and the search term straight into the string. The `org_id` part is not attacker-controlled, but the `q` parameter is. The search selects three columns, so an injected `UNION SELECT` with three columns, id then email then the password hash, lines up against the result shape and returns the full users table through the invoice search response. Escaping quotes is the wrong fix and interviewers know it; the fix is to let the driver bind the values: ``` @app.get("/invoices/search") def search_invoices(): user = current_user() q = request.args.get("q", "") sql = ("SELECT id, customer, total FROM invoices" " WHERE org_id = ? AND customer LIKE ?") rows = db().execute(sql, (user["org_id"], f"%{q}%")).fetchall() return jsonify([dict(r) for r in rows]) ``` One strong-pass detail: after this change, a `%` or `_` in `q` still acts as a LIKE wildcard. That is a correctness point, not injection: worth a sentence, not a block. Path traversal is the second critical, and it is nastier than it looks because of what the attachments directory sits next to. Line 59 joins the user's `name` onto the upload directory with `os.path.join`, which happily walks upward on `../app.db` and throws the base path away entirely on an absolute path. Reading `app.db` hands over every tenant's data. On Linux the attacker learns the working directory from `/proc/self/cwd` or `/proc/self/cmdline`, then reads the source for the signing key on line 9; reading `/proc/self/environ` hands over the admin token from the process environment. `secure_filename` alone is not the answer, because it mangles legitimate names; the clean fix refuses traversal while serving from the one directory: ``` from flask import send_from_directory @app.get("/attachments") def download_attachment(): current_user() return send_from_directory(UPLOAD_DIR, request.args["name"]) ``` That stops the traversal, but it does not add authorization. Any logged-in user can still download any tenant's attachment by guessing the name, which is one of the below-the-line findings. SSRF is the third, and the reason it is a security-engineer favorite is that the code looks like an ordinary feature. Line 64 fetches whatever URL the caller sends. On an AWS instance with IMDSv1 enabled, pointing it at the metadata role-credentials path makes the server fetch its own temporary keys, write them to the attachments directory, and then a normal download reads them back. This is the same shape as [SSRF against the cloud metadata endpoint](https://www.techinterview.org/post/3233477308/ssrf-cloud-metadata-interview/), where IMDSv2's required token and the equivalent header checks on GCP and Azure are what blunt a bare GET. The naive fix, resolving the host once with `gethostbyname` and checking that one address, has a real gap: `requests` resolves again with `getaddrinfo`, which can return more addresses, so a host with one public record and one pointing at loopback passes the check and then connects to loopback. Validate every resolved address, handle an unresolvable name without a 500, follow redirects yourself and re-check each hop, and cap the body: ``` import ipaddress, socket from urllib.parse import urljoin, urlparse MAX_BYTES = 2 * 1024 * 1024 def all_public(host): try: infos = socket.getaddrinfo(host, None) except socket.gaierror: return False addrs = {i[4][0] for i in infos} return bool(addrs) and all(ipaddress.ip_address(a).is_global for a in addrs) def safe_get(url, hops=3): for _ in range(hops): parts = urlparse(url) if parts.scheme not in ("http", "https") or not parts.hostname: abort(400) if not all_public(parts.hostname): abort(400) resp = requests.get(url, timeout=5, allow_redirects=False, stream=True) if not resp.is_redirect: body = resp.raw.read(MAX_BYTES + 1, decode_content=True) if len(body) > MAX_BYTES: abort(413) return body url = urljoin(url, resp.headers["location"]) abort(400) ``` ``` @app.post("/invoices//logo") def fetch_logo(invoice_id): user = current_user() inv = one("SELECT id FROM invoices WHERE id = ? AND org_id = ?", invoice_id, user["org_id"]) if inv is None: abort(404) content = safe_get(request.get_json()["url"]) name = f"logo-{invoice_id}.png" with open(os.path.join(UPLOAD_DIR, name), "wb") as f: f.write(content) return jsonify(attachment=name, bytes=len(content)) ``` Even validating every resolved address narrows the window rather than closing it, because the HTTP client resolves the name a second time. The complete fix resolves once and pins the connection to that validated IP, carrying the original Host header and, over HTTPS, the SNI name so the certificate still verifies, or it routes all outbound fetches through an egress proxy with a destination allowlist. Saying that out loud is a strong-pass move: it shows you know the address check is necessary but not sufficient. The scoped invoice lookup in the fix above also closes a separate bug, the cross-tenant logo overwrite covered below. ## The five that round out the review The hard-coded `secret_key` on line 9 is a High and a multiplier. Flask signs session cookies with it, so a known key means a forged session for any user; in a test it takes one call to Flask's own cookie serializer to mint a valid `session` cookie as the victim. That is strictly broader than reading one tenant's invoices, which is why it ranks above the object-level bug. It does not, though, open `/admin/purge`: that route is gated by the `X-Admin-Token` header on line 10, not by the session, and that token is read from the environment, which is the right pattern. The contrast is worth naming, because the file already knows how to keep a secret out of source and just did not do it for the signing key. The object-level bug on line 42 reads any invoice by id; the fix scopes the lookup to the caller's `org_id` the way the search endpoint already does. These are where most [API security](https://www.techinterview.org/post/3233460404/api-security-interview-questions/) incidents actually live, not in exotic payloads. The verbose 500 handler and the missing login throttle are both Medium and both easy to under-rate. The traceback handler returns exactly the map an attacker wants for the next step: the filesystem paths, the stack frames, and the database's own error text confirming a query broke. The login endpoint has no backoff, so it is a credential-stuffing target; the fix is a [rate limiter](https://www.techinterview.org/post/3233474159/system-design-rate-limiter-token-bucket-sliding-window-leaky-bucket-distributed-rate-limiting-api-gateway/) keyed on both the account and the source IP, with lockout or exponential backoff. The non-constant-time `!=` on the admin token is the Low, and the right way to raise it is plainly: it is the wrong primitive, `hmac.compare_digest` is the one-line fix, and a remote timing attack across a network is hard enough that you would not block a release on it alone. The [OWASP Top 10 and web security](https://www.techinterview.org/post/3233474144/system-design-web-security-owasp-xss-csrf-sql-injection-cors-csp-authentication-authorization-jwt-oauth/) rundown covers the background. ## Lower-severity bugs to name after the criticals The login handler on line 34 short-circuits on an unknown email and never calls `check_password_hash`, so an unknown address returns in well under a millisecond while a real one pays for the scrypt check. That timing gap enumerates valid accounts, and it is easier to exploit than the admin-token timing bug that made the ranked list; the fix is to compare against a dummy hash for unknown users so both paths do the same work. The `fetch_logo` endpoint on line 62 checks that you are logged in but not that `invoice_id` is yours, so one tenant can overwrite another tenant's logo, which the scoped lookup in the SSRF fix closes. And `send_from_directory` stops traversal but adds no authorization, so any logged-in user still downloads any tenant's attachment by name; the staff-level answer is to store attachments with an owner and serve them by id scoped to `org_id`, never by raw filename. CSRF is worth one sentence to show you weighed it: the JSON endpoints reject a cross-site form post because `get_json` returns a 415 on a non-JSON content type, so there is no classic CSRF here, though that is a side effect rather than a control you would rely on. ## Where candidates lose the round Two failure modes sink otherwise strong engineers. The first is leading with style. A candidate opens with "you are using an old string-formatting style" or "there are no type hints" and spends four minutes on lint before touching the injection. The interviewer has already decided. Nits are fine at the end, as a quick sweep, never first. The second is finding the bug and never stating the impact. "Line 59 is path traversal" earns a nod; stopping there earns a shrug. The sentence that follows, who gets what from what starting position, is the entire point of the round, because in the real job your finding competes for a release slot against a product manager who wants to ship. If you cannot say what an attacker gets, the fix does not get prioritized. The quiet third failure is treating the exercise as a quiz with a fixed answer key, which is what memorizing lists of security code review interview questions trains you to do. Good reviewers ask the interviewer questions: is this behind an authenticated gateway, is the upload directory a shared mount, does the org model allow sub-orgs. The planted bugs do not change, but how you rank them does, and an interviewer who watches you adjust severity to the deployment context is watching a security engineer work, not a candidate reciting OWASP.