Post cover image

July 23, 2026

Secure Code Review Challenge #1: Schooled β€” Solution (One Whitespace Character Away From Admin)

πŸ“’ The solution to Challenge #1: Schooled is live. Watch the video walkthrough here, or read the full write-up on GitHub.

By Mohamed AboElKheir

9 min read

πŸ“’ The solution to Challenge #1: Schooled is live. Watch the video walkthrough here, or read the full write-up on GitHub.

The Secure Code Review Challenge is a free biweekly series of full, realistic applications with vulnerabilities based on real-world CVEs β€” you review, identify, and exploit them the way you would in a real security review, not just spot-the-bug pattern recognition.

If you haven't attempted the challenge yet, this is your cue to stop reading, clone the repo, and try it yourself first. Everything below assumes you've already had a go at it β€” no shame either way, but the exercise is worth more if you struggle with it a bit before seeing the answer.

Two quick announcements before we get into it:

  • Challenge #2 is already live in the repo under challenges/. The solution to it will follow in a couple of weeks, alongside a third challenge.
  • The repo now uses GitHub Releases for every new challenge and solution drop. If you go to Watch β†’ Custom β†’ Releases on the repo, you'll get notified automatically instead of having to check back manually.

With that out of the way, let's walk through Schooled the same way I did in the video β€” following the same seven-step methodology laid out in the repo, end to end.

A Quick Reminder of What We're Reviewing

Schooled is a small course-management platform. Teachers create and manage courses, students browse and enroll in them, and admins manage the platform and its users. Nothing exotic β€” which is exactly the point. Most real applications you'll review in your career look like this: a handful of roles, a handful of routes, and a database behind it. The vulnerability we're after isn't hiding behind obscure architecture; it's hiding in a very ordinary-looking piece of logic.

Part I β€” Building the Mental Model

1. Application Scope & Architecture

Before touching any code, it's worth just running the app and using it like a normal user would β€” this is exactly what the challenge README walks you through with a single docker compose up --build. Register as a teacher, create a course, log out, register as a student, enroll in that course. Five minutes of clicking around tells you more about the intended behavior than staring at the code cold.

Once you're through that, it's time to actually read the stack:

  • Node.js + Express for the backend routes
  • EJS as the server-rendered templating engine for the UI
  • PostgreSQL, accessed through the pg library
  • JWT for authentication, stored in an httpOnly, SameSite=Strict cookie for the browser UI, or passed as a Bearer token for API clients

The docker-compose.yml is a great entry point into the architecture itself β€” it tells you there are exactly two containers (the app and Postgres), and that the app's Dockerfile builds from a Node base image and boots via an npm start script, which in turn runs node server.js. From server.js, you can see three route groups get attached β€” /api/auth, /api/courses, and /api/admin β€” plus a single EJS-rendered page and a public/app.js file that runs client-side.

That's enough to have a working mental model: Express handles routing, EJS renders the one HTML page, app.js handles DOM updates in the browser, and everything durable goes through a single database class using parameterized pg queries.

2. Entry Points

With the architecture in hand, enumerating entry points is mostly a matter of reading the route files:

  • POST /api/auth/register β€” No auth required. Body: username, email, password, role
  • POST /api/auth/login β€” No auth required. Body: username, password
  • GET / β€” No auth required (cookie optional). Renders the EJS UI
  • GET /api/courses β€” Any authenticated user. Lists courses
  • GET /api/courses/:id β€” Any authenticated user. :id from URL
  • GET /api/courses/user/enrollments β€” Any authenticated user. Returns own enrollments
  • POST /api/courses β€” teacher or admin. Creates a course
  • POST / DELETE /api/courses/:id/enroll β€” student or admin. Enroll / unenroll
  • GET /api/admin/users β€” admin only. Lists all users
  • GET /api/admin/users/:id β€” admin only. Gets a single user
  • POST /api/admin/users/:id/promote β€” admin only. Promotes a user to admin

Two things are worth flagging immediately: the web UI surfaces all of this to a browser, which means it's an attack surface for XSS and CSRF even though it hits the same backend routes; and the only documented way to become an admin is either being the seeded admin account or being promoted by an existing one via /api/admin/users/:id/promote. Keep that second point in your back pocket β€” it matters a lot later.

3. Dangerous Sinks

Anywhere user input can end up somewhere dangerous:

  • SQL queries β€” all through the pg Pool in src/store/database.js
  • HTML rendering β€” the EJS template (views/index.ejs), which outputs user.username and user.role from the JWT
  • DOM mutation β€” public/app.js, which takes API JSON responses (usernames, emails, roles, course data) and writes them into the page

4 & 5. Threat Modeling and Mitigation Review

This is where most of the actual review time goes, and it splits cleanly into the two buckets from the methodology: business-logic vulnerabilities and source-to-sink vulnerabilities.

Business logic first. The obvious candidates here are broken authentication, broken authorization, and β€” because of what we noted in the entry points step β€” how role assignment actually works.

Authentication turned out to be solid: an authenticateToken middleware checks either the Authorization header or the cookie, verifies the JWT signature, and re-fetches the user from the database on every request (so a revoked user can't keep using a still-valid token). It's attached to every course and admin route.

Role-based authorization also looked fine on inspection β€” a requireRole middleware gates the admin routes, and role never comes from anything other than the authenticated user's DB record.

IDOR was a natural thing to check next, since we have resource IDs (courses, users) flowing through URLs. But in this app, courses have no private/ownership model β€” every course is visible to every authenticated user regardless of ID β€” so there's no meaningful authorization boundary to bypass there. Worth checking, low value once you understand the intended behavior, so it's fine to move past it quickly.

Then there's the piece flagged back in step 2: role assignment at registration. Only an existing admin can promote someone, and the registration endpoint is supposed to block you from just registering as admin outright. On the surface, both of those checks are present in the code. This is the point in the review where it's tempting to conclude "authorization looks fine" and move on β€” and that's exactly the trap the challenge is built around.

Source-to-sink next. Each of the sinks from step 3 gets checked against how it's actually being fed:

  • XSS via EJS β€” the template uses <%= %>, which HTML-escapes <, >, ", and & automatically. A <script> tag in a username renders as inert text.
  • XSS via client JS β€” app.js uses textContent and element properties exclusively, never innerHTML. Per the MDN docs, textContent doesn't parse its input as HTML at all, so there's no injection path here even with attacker-controlled API data.
  • CSRF β€” the auth cookie is set with SameSite=Strict. A forged cross-site request simply won't carry the cookie, so it never reaches an authenticated context. (Bearer-token API clients aren't affected by CSRF in the first place.)
  • SQL injection β€” every query in database.js uses parameterized placeholders ($1, $2, …) rather than string concatenation, so there's no way to break out of the query structure.

At this point, every source-to-sink vector checks out clean, and the business-logic checks all appear to check out too. This is usually the moment where a review either stops (and misses something) or where you go back and look harder at the piece that felt slightly too convenient.

Part II β€” Finding, Exploiting, and Fixing the Bug

6. The Vulnerability: Two True Statements That Aren't Both Enforced the Same Way

The role-assignment logic rests on two claims:

  1. You cannot register as admin.
  2. Only an existing admin can promote another user to admin.

The second one holds up cleanly β€” it's just requireRole('admin') on the promote route, verified against the authenticated user's role from the JWT. Nothing subtle there.

The first one is where it falls apart, and the reason is a mismatch between where a check happens and where a transformation happens.

In src/routes/auth.js, the registration handler rejects the request outright if role === 'admin' β€” a straightforward denylist check against the raw input. But by the time that value reaches the database layer, createUser() (and the VALID_ROLES allowlist that runs just before it) both call role.trim() before using the value. So the guard checks one representation of the input, and the storage layer commits a different representation.

Send "admin " β€” with a trailing space β€” and here's what happens:

  • The guard compares "admin " === "admin" β†’ false. No rejection.
  • VALID_ROLES.includes(role.trim()) β†’ "admin ".trim() is "admin", which is in the allowlist. Passes.
  • createUser() trims again before the INSERT, storing the literal string "admin".
  • The JWT returned in the registration response is built from that DB row, so it already carries role: "admin".

You never touch the promotion endpoint at all. One request, one trailing space, immediate admin.

Proof of concept:

# 1. Register with a trailing space in the role
TOKEN=$(curl -s -X POST http://localhost:3000/api/auth/register \
  -H "Content-Type: application/json" \
  -d '{"username":"attacker","email":"attacker@evil.com","password":"pass123","role":"admin "}' \
  | jq -r '.token')
# The response's user.role is already "admin" - the token encodes that role.
# 2. Immediately call an admin-only endpoint
curl -s -X GET http://localhost:3000/api/admin/users \
  -H "Authorization: Bearer $TOKEN" | jq .
# β†’ returns the full user list, confirming admin access

Doing this through Burp against the actual UI is just as easy: intercept the registration request, change "role":"admin" to "role":"admin ", forward it, and log in as that new user. The response comes back without the "cannot register as admin" error, and the account already has admin privileges β€” no promotion step required.

A few other payloads work identically, since they all reduce to "admin" once trimmed:

  • "admin " (trailing space)
  • " admin" (leading space)
  • "admin\t" (tab)
  • " admin " (both)

One thing that's worth calling out explicitly, because it's easy to assume otherwise: "Admin" (capitalized) does not work here. The allowlist check (VALID_ROLES.includes(role.trim())) is case-sensitive, so "Admin".trim() is still "Admin", which isn't in the allowlist β€” that variant gets rejected, almost by accident, because case-folding wasn't part of the transformation chain in this particular codebase. It's a useful reminder that these bugs are about a specific mismatched transformation, not about "any weird string bypasses everything."

7. The Fix

The core problem is a check performed on one representation of a value and a write performed on another. The fix is to make sure both operate on the same, already-normalized value β€” and to flip the registration check from a denylist to an allowlist while we're at it, since denylists only work as well as the list of things you thought to deny:

const { username, email, password, role } = req.body;
if (!username || !email || !password || !role) {
  return res.status(400).json({ error: 'All fields are required' });
}
const normalizedRole = role.trim().toLowerCase();
const allowedRoles = ['student', 'teacher'];
if (!allowedRoles.includes(normalizedRole)) {
  return res.status(403).json({ error: 'Cannot register as admin' });
}
// Pass the already-normalized value downstream so storage stays consistent
const user = await db.createUser(username, email, passwordHash, normalizedRole);

Now the exact same value that gets checked is the one that gets stored β€” there's no longer a gap for a transformation to open up between the two.

A few defense-in-depth options worth layering on top, in case the application-layer fix is ever missed or bypassed elsewhere:

  • A database CHECK constraint on the role column (CHECK (role IN ('student', 'teacher', 'admin'))), which would have stopped this exploit even without touching the application code at all.
  • Remove the trim() call from createUser. Normalization belongs at the boundary β€” the route handler β€” not silently inside the data-access layer. That's exactly where this gap was introduced.
  • Derive the JWT role from the normalized value, so it can never diverge from what was actually validated.

8. Why This Matters Beyond Whitespace

The specific bug here is a trailing space, but the pattern is much bigger than that, and it's the main thing I want people to take away from this challenge: whenever a security check runs against one representation of an input, and a later transformation produces a different representation that actually gets used or stored, you have a gap β€” regardless of what that transformation is. Whitespace trimming is just the easiest one to demonstrate. The same class of bug shows up with:

  • Case folding β€” checking role !== 'admin', then lowercasing (or hitting a case-insensitive DB collation) downstream.
  • Unicode normalization β€” NFC/NFD normalization or full-width/half-width folding turning a fullwidth "ａdmin" into "admin" after the check already passed.
  • URL/percent-decoding β€” validating a raw parameter, then decoding it again later, letting something like double-encoded input resolve into a disallowed value.
  • Path canonicalization β€” resolving .. or redundant separators after a path has already been allow/deny-checked (classic path traversal territory).
  • Type coercion β€” validating a string form of a value, then having a downstream layer coerce it into something a loosely-typed language interprets differently.
  • Deduplication β€” collapsing repeated characters or delimiters after the check has already run.

The fix pattern is the same no matter which transformation is involved: normalize first, validate the normalized form, and pass that same normalized value everywhere downstream. Never validate one representation of a value and act on another.

This isn't a theoretical concern, either. HashiCorp Vault has had multiple real authentication and authorization flaws rooted in exactly this class of discrepancy β€” different components handling the same input inconsistently. If you want to see this pattern play out in a production-grade secrets manager, Cyata's write-up on cracking Vault's authentication and authorization is a great follow-up read. It's also cataloged as CWE-179: Incorrect Behavior Order: Early Validation if you want the formal classification, alongside the OWASP Authorization Cheat Sheet and OWASP Input Validation Cheat Sheet for general guidance on avoiding it.

Wrapping Up

If you worked through Schooled yourself, I'd genuinely like to know how your threat model compared β€” did you catch the role-assignment gap on the first pass, or did it take a second look after everything else checked out clean? Drop your thoughts in the comments, or let me know if you think we missed anything in the review.

Challenge #2 is live now if you're ready for the next one, and I'll be back in a couple of weeks with its solution and a new challenge alongside it.

Links:

None