AppSec Engineer Code and Weakness Review Drills

Purpose: sharpen the interview skill of reading unknown code fast, spotting security weaknesses without scanners, and explaining fixes in a way that sounds calm, structured, and credible.

How this page is different from the broader secure-coding packs: these drills are written as interview exercises, not as a language tutorial. Each snippet intentionally contains multiple issues and a few distracting lines.

Companion page

For a denser by-language screen of dangerous functions, keywords, sink patterns, and "what to say first" guidance, use AppSec Vulnerable Code Screening Cheat Sheet by Language.

Use this mental model first

When an interviewer drops a code snippet on the screen, do not start line-by-line reading from top to bottom. Use this order instead:

1. Identify the trust boundary first. Where does attacker-controlled input enter the code? HTTP request, headers, cookies, query params, file uploads, environment variables, DB results, webhook payloads, CLI args, deserialized data.
2. Look for the dangerous sinks. SQL execution, OS command execution, file system access, template rendering, deserialization, crypto calls, authn/authz decisions, redirect logic, archive extraction, object mapping, shell helpers, dynamic evaluation.
3. Check auth before business logic. Many candidates jump straight to injection and miss broken authorization, tenant isolation, or ownership validation.
4. Scan for state transitions. Password reset, role change, money movement, promo redemption, approval, delete/export, admin actions, token issuance, session creation.
5. Only then read the implementation details. Validate assumptions, note exact weak lines, propose fixes, then mention compensating controls and tests.

Quick verbal pattern for a strong answer

A strong AppSec answer usually sounds like this:

"I would start by identifying untrusted inputs and the dangerous sinks. Here I see user-controlled data reaching [sink] without [control]. The primary issue is [weakness]. The likely impact is [impact]. I would fix it by [precise fix], then add [test, lint rule, review check, defense in depth] so the issue does not come back."

What to skip at first pass

Do not waste your first 60 seconds on:

• style nits that do not change security semantics;
• constant definitions and harmless DTOs;
• comments that do not affect control flow;
• logging noise unless it leaks secrets or enables tampering;
• import order or formatting unless it hints at a dangerous library.

Drill 1 - Python API handler

from flask import request, jsonify
import os
import sqlite3
import hashlib

conn = sqlite3.connect("users.db", check_same_thread=False)

@app.route('/admin/user/search')
def search_user():
    user = request.args.get('user', '')
    sort = request.args.get('sort', 'name')
    token = request.headers.get('X-API-Token')

    if token == os.getenv('ADMIN_TOKEN'):
        query = f"SELECT id, name, email, role FROM users WHERE name LIKE '%{user}%' ORDER BY {sort}"
        rows = conn.execute(query).fetchall()
        return jsonify(rows)
    return jsonify({'error': 'forbidden'}), 403

@app.route('/admin/user/reset-password', methods=['POST'])
def reset_password():
    data = request.json
    new_pw = hashlib.md5(data['password'].encode()).hexdigest()
    conn.execute(f"UPDATE users SET password='{new_pw}' WHERE id={data['id']}")
    conn.commit()
    return {'ok': True}

What is wrong

1. SQL injection in user. The LIKE '%{user}%' pattern is direct string interpolation.
2. SQL injection in sort. Interview candidates often miss ORDER BY injection. Column names must be allow-listed.
3. Static admin token as sole control. A shared header token is not strong admin authentication and has no user attribution.
4. Broken authorization on password reset. The second endpoint performs a high-risk state change without checking actor identity, role, or target ownership.
5. Weak password hashing. MD5 is obsolete and unsuitable for password storage.
6. Potential secret leakage / weak operational model. A single environment token encourages unsafe sharing, hard rotations, and poor auditability.

How to explain it in interview language

"I see both classic SQL injection and a less obvious structural injection in the ORDER BY clause. I also see a design problem: a shared header token is being used as an admin identity, which removes attribution and makes least privilege hard. The password reset path is the higher-severity business action because it lacks actor validation and uses MD5 for password storage."

Safer direction

ALLOWED_SORTS = {'name': 'name', 'email': 'email', 'role': 'role'}

@app.route('/admin/user/search')
def search_user():
    actor = require_authenticated_admin(request)
    user = request.args.get('user', '')
    sort = ALLOWED_SORTS.get(request.args.get('sort', 'name'), 'name')
    rows = conn.execute(
        f"SELECT id, name, email, role FROM users WHERE name LIKE ? ORDER BY {sort}",
        (f"%{user}%",)
    ).fetchall()
    audit_log(actor, 'admin_user_search', {'search_term': user, 'sort': sort})
    return jsonify(rows)

And for password hashing use bcrypt, scrypt, or Argon2 through a reputable library.

Bonus points to mention

• Add integration tests for injection payloads in query params and sort.
• Add audit logging for admin actions.
• Rate-limit and alert on repeated reset operations.
• Use per-user authn, not a shared secret header.

Drill 2 - PHP file export + template rendering

<?php
session_start();

if ($_SESSION['is_admin']) {
    $name = $_GET['name'];
    $type = $_GET['type'];
    $tpl = $_GET['tpl'];

    $file = "/var/reports/" . $name . "." . $type;
    header("Content-Type: text/html");

    if (file_exists($file)) {
        echo file_get_contents($file);
    }

    include "templates/" . $tpl . ".php";
}

What is wrong

1. Weak session trust. $_SESSION['is_admin'] is used as a raw boolean without re-checking session freshness, identity, or CSRF-sensitive context.
2. Path traversal / arbitrary file read. name and type are concatenated into a file path without canonicalization or allow-listing.
3. Local file inclusion risk. tpl controls include path. That is dangerous even if .php is appended.
4. Potential XSS / content confusion. Everything is returned as text/html, even exported report content.
5. No tenant or object-level authorization. Adminness alone does not necessarily authorize access to every report.

Better answer framing

"The first thing I notice is user-controlled path construction in both file access and template include. That creates traversal and local file inclusion risk. The session check is also too thin for a high-risk admin operation, and the content type forces browser rendering of attacker-influenced content, which can turn a file-read bug into an XSS delivery vector."

Safer direction

• Replace name, type, and tpl with allow-listed identifiers.
• Resolve files from a server-side metadata lookup, not direct path concatenation.
• Use readfile() or streaming with correct content type, not text/html by default.
• Add CSRF protection and a stronger admin session model.

$allowedTemplates = ['summary', 'daily', 'weekly'];
$tpl = $_GET['tpl'] ?? 'summary';
if (!in_array($tpl, $allowedTemplates, true)) {
    http_response_code(400);
    exit('invalid template');
}

Drill 3 - Java Spring controller

@PostMapping("/tenant/{tenantId}/invoice/{id}/approve")
public ResponseEntity<?> approve(
        @PathVariable String tenantId,
        @PathVariable Long id,
        @RequestBody ApprovalRequest body,
        Principal principal) {

    Invoice inv = invoiceRepository.findById(id).orElseThrow();

    if (body.isApproved()) {
        inv.setStatus("APPROVED");
        inv.setApprovedBy(principal.getName());
        invoiceRepository.save(inv);
    }

    return ResponseEntity.ok(inv);
}

What is wrong

1. IDOR / cross-tenant authorization gap. The code loads the invoice by id only and never verifies tenantId ownership.
2. Missing role check / workflow authorization. Any authenticated principal may be able to approve invoices.
3. No state-machine validation. It may allow approval from invalid states.
4. No dual control / business policy enforcement. High-risk approvals often require separation of duties.
5. Over-sharing in response. Returning the whole invoice object can leak fields unnecessarily.

Strong interview phrasing

"This is not an injection bug, it is an object-level authorization and workflow-control bug. The path contains tenantId, but the lookup ignores it. That means a user from tenant A might approve tenant B's invoice if they know the ID. I would treat this as a high-severity cross-tenant control failure."

Safer direction

@PreAuthorize("hasAuthority('invoice:approve')")
@PostMapping("/tenant/{tenantId}/invoice/{id}/approve")
public ResponseEntity<?> approve(...) {
    Invoice inv = invoiceRepository.findByIdAndTenantId(id, tenantId)
        .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND));

    approvalPolicy.assertActorCanApprove(principal.getName(), inv);
    approvalPolicy.assertValidStateTransition(inv, InvoiceStatus.PENDING_REVIEW, InvoiceStatus.APPROVED);

    inv.setStatus("APPROVED");
    inv.setApprovedBy(principal.getName());
    invoiceRepository.save(inv);
    return ResponseEntity.ok(Map.of("id", inv.getId(), "status", inv.getStatus()));
}

Nice follow-up points

• Mention unit tests for same-tenant and cross-tenant scenarios.
• Mention domain events / immutable audit trail for approvals.
• Mention four-eyes approval if business risk is high.

Drill 4 - C++ archive extraction utility

#include <filesystem>
#include <fstream>
#include <cstdlib>
using namespace std;

void extractUserArchive(const string& archiveName, const string& username) {
    string base = "/srv/uploads/" + username + "/";
    filesystem::create_directories(base);
    string cmd = "tar -xf /tmp/" + archiveName + " -C " + base;
    system(cmd.c_str());

    ofstream audit(base + "extract.log", ios::app);
    audit << "archive=" << archiveName << "\n";
}

What is wrong

1. Command injection. archiveName is interpolated into system().
2. Archive extraction risk / Zip Slip style path traversal. Even if the command were safe, archive entries may escape base via ../ or symlinks.
3. User path trust. username is used in a filesystem path without validation.
4. Local log tampering. The audit log is written inside the user-controlled extraction directory.
5. No resource controls. Archive bombs and disk exhaustion are possible.

What a mature answer sounds like

"The immediate bug is command injection, but the more complete answer is that archive extraction itself is a dangerous sink. Even replacing system() would not solve traversal through crafted archive members, symlinks, or decompression bombs. Also, the audit record is written into the same mutable path, so an attacker may erase or overwrite the evidence."

Safer direction

• Use a safe archive library instead of shelling out.
• Normalize and validate target paths for every archive member.
• Reject symlinks or special files unless explicitly needed.
• Enforce extraction quotas and timeouts.
• Write audit logs to append-only central logging, not the user directory.

Drill 5 - SQL review exercise

CREATE PROCEDURE GetOrders(IN p_user_id INT, IN p_sort VARCHAR(50))
BEGIN
  SET @q = CONCAT(
    'SELECT id, amount, status, card_last4 FROM orders WHERE user_id = ',
    p_user_id,
    ' ORDER BY ',
    p_sort
  );
  PREPARE stmt FROM @q;
  EXECUTE stmt;
END;

What is wrong

1. Dynamic SQL with untrusted ordering field. p_sort is structural injection.
2. Potential user ID trust issue. If application authn/authz is weak, p_user_id can become an IDOR-style data access issue.
3. Sensitive data exposure. card_last4 may be allowed, but interviewers want to know whether it is necessary in this query path.
4. No row-level security beyond passed parameter. The DB layer trusts the caller to pass the correct user ID.
5. Stored procedure does not equal safety. Many candidates assume stored procedures are automatically safe.

Good verbal answer

"Stored procedures are not magic. This procedure still builds dynamic SQL, so the ORDER BY field remains injectable unless it is allow-listed. I would also question why the caller provides p_user_id at all if identity is already known upstream. That creates another chance for broken access control."

Safer direction

• Replace freeform p_sort with enumerated values.
• Keep row ownership enforced in the application and, where possible, in the database access model.
• Return only necessary columns.

Drill 6 - TypeScript / Express auth shortcut

app.post('/api/admin/create-user', async (req, res) => {
  const actor = req.user as any;
  const body = req.body;

  if (actor && actor.role == 'admin' || actor.permissions.includes('user:create')) {
    const user = await prisma.user.create({
      data: {
        email: body.email,
        role: body.role,
        passwordHash: body.passwordHash,
        isMfaRequired: false
      }
    });
    res.json(user);
    return;
  }

  res.status(403).json({error: 'forbidden'});
});

What is wrong

1. Operator precedence bug. && and || grouping can produce surprising access behavior if actor is missing or malformed.
2. Mass-assignment / trust of caller-controlled privilege fields. role and passwordHash are taken directly from the body.
3. Security control bypass by design. isMfaRequired is hard-coded to false.
4. Potential null dereference / auth middleware contract weakness. actor.permissions may throw if actor is not what is expected.
5. Returns created user object directly. May expose internal fields.

Strong answer pattern

"I see a control-plane endpoint where the server trusts too much client-provided security metadata. Even if the authorization condition were fixed, letting the caller supply role and passwordHash is unsafe. Administrative user creation should use a server-owned workflow with explicit allowed roles, password handling on the server, and defaults that do not weaken MFA."

Safer direction

const canCreateUser = !!actor && (
  actor.role === 'admin' || actor.permissions?.includes('user:create')
);
if (!canCreateUser) return res.status(403).json({ error: 'forbidden' });

const allowedRoles = new Set(['viewer', 'analyst']);
if (!allowedRoles.has(body.role)) return res.status(400).json({ error: 'invalid role' });

const passwordHash = await argon2.hash(body.password);

Final cheat sheet - what to say when you are not 100% sure

If you are not fully certain, do not freeze. Say:

• "The exact framework API may vary, but the security issue is that untrusted input reaches a dangerous sink without a control."
• "Even if this specific line is safe in a narrow sense, the design still trusts client-controlled security metadata too much."
• "I would verify whether there is an upstream control, but absent proof of that, I would treat this as a weakness."
• "The bug class I suspect here is broken object-level authorization / injection / unsafe deserialization / path traversal, and I would test it this way..."

Cross-links

• AppSec Engineer Interview Pack (2026)
• Live Code-Review Drills and Answer Guides
• Secure Coding Review Labs and Language Checklists
• Business Logic Vulnerabilities and Verification