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/ - no@authenticate_token decorator at all.id comes from the URL - PUT /profiles/ - authenticated. Body:bio ,work_experiences[] ,is_private , plusid from the URL - DELETE /profiles/ - 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/ 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 ) acrossstore.py , fed by usernames and profile fields - ObjectId(profile_id) , fed by the URL segment - DOM rendering in the frontend, fed by whatever the API returns as JSON - ReportLab's Paragraph() markup parser, fed bybio , work-experience fields, andusername 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 , , and . 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/ ? 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 Med