iframe Sandbox Lessons From the ChatGPT Account Takeover
I have a confession about iframes. For years I treated the sandbox attribute like a seatbelt: I clicked it in, felt safe, and never once asked what the car was doing. Then I read Simon Willison's link post about how researchers took over ChatGPT accounts and realised the seatbelt was fine. The problem was a door nobody had locked, three feet away.
Short version for the impatient: the sandbox held. The attack worked because the parent page had a message handler that wrote whatever it received into the DOM with innerHTML, and it never asked who was talking. If your app embeds untrusted HTML and also listens for postMessage, that handler is the thing to audit this week, not the iframe attribute.
I'll walk through the chain as Simon summarised it, then show the handler pattern I now use in my own code. I'm working from his write-up and not from the original researchers' report, so treat my account of the details as a summary and go read the sources yourself.
What the attack chain actually did
ChatGPT lets you preview interactive HTML apps in a sandboxed iframe. That's the same idea as the artifacts pattern you may have seen elsewhere. The point of the sandbox is that the HTML inside is untrusted. It might be written by a stranger, shared through a link, and opened by you while signed in.
According to Simon's summary, the researchers at Parameter Labs found a chain of small problems. The parent page used the MathLive library for graph support, and MathLive ships its own virtual keyboard. That keyboard can be triggered in the parent page through postMessage(). The receiving code used innerHTML, so anything able to send a message could push HTML into the parent. Nothing stopped a sandboxed preview frame from sending one.
Inserting HTML is not the same as running script. innerHTML won't execute a <script> element you hand it. But it will happily create an <iframe>, and a new iframe created by the parent has no sandbox unless the parent adds one. So the attacker got an unsandboxed frame inside the page.
Then came the content security policy. It blocked inline scripts and only allowed scripts from a short list of domains. One of those domains was YouTube, and the YouTube oEmbed endpoint could be coaxed into reflecting JavaScript back in a form that ran. With script running on the ChatGPT origin, fetching private data from the APIs and sending it out through an image request was, in Simon's word, trivial. Share a link, wait for a signed-in person to open it, collect their data.
I find the shape of this more useful than the specific bug. Four separate decisions, each defensible in a code review on its own, added up to account takeover. A message handler that trusts its sender. A DOM write that accepts markup. A frame that inherits no restrictions. An allowlist entry for a big, friendly domain. Remove any single one and the chain breaks.
Why the sandbox attribute wasn't the problem
The sandbox attribute did its job. A sandboxed frame without allow-same-origin gets an opaque origin, so it can't read the parent's cookies or storage, and it can't touch the parent's DOM directly. What it can still do, if you let it run scripts, is call window.parent.postMessage(). That channel is deliberate. It's the supported way for a restricted frame to talk to the page around it.
So the real question is what your page does when a message arrives. Think of that handler as a public API endpoint, because functionally it is one. Any frame that can reach your window can call it, including frames you never meant to host.
Here's the pattern that worries me:
// Before: trusts every message from every frame
window.addEventListener('message', (event) => {
const { type, html } = event.data;
if (type === 'show-keyboard') {
document.getElementById('kb').innerHTML = html;
}
});
Nothing here checks event.origin. Nothing checks event.source. The payload shape is whatever the sender says it is. And the innerHTML write means the sender controls markup, which is the exact step the researchers needed.
A handler I would actually ship
The MDN documentation for postMessage is blunt about this: always verify the sender's identity using the origin and possibly source properties, and be wary of what you do with the data. Here's my current version, rewritten after reading the ChatGPT write-up:
// After: known sender, known shape, no markup from outside
const KEYBOARD_ORIGIN = 'https://app.example.com';
const keyboardFrame = document.getElementById('keyboard-frame');
window.addEventListener('message', (event) => {
if (event.origin !== KEYBOARD_ORIGIN) return;
if (event.source !== keyboardFrame.contentWindow) return;
const data = event.data;
if (!data || data.type !== 'show-keyboard') return;
if (typeof data.layout !== 'string') return;
const layouts = { numeric: 'numeric', text: 'text' };
const layout = layouts[data.layout];
if (!layout) return;
document.getElementById('kb').dataset.layout = layout;
});
Four things changed. The handler compares the origin against one exact string, not a prefix and not endsWith. It also compares event.source to the specific frame I expect, because two different frames can share an origin. The payload is validated against a closed set of values. And nothing from the message ever becomes markup. It becomes a data- attribute that my own CSS reads.
That last change is the one that matters most. If a handler needs to render something the sender supplied, use textContent, or build elements with createElement. If it genuinely has to accept markup, run it through a sanitizer such as DOMPurify and then think hard about whether you really needed markup.
One more trap with sandboxed frames: their origin is the string "null". If you ever write if (event.origin === 'null') to accept messages from a sandboxed preview, you've just accepted messages from every sandboxed frame on the internet. It's an easy line to write when a preview pane refuses to load, so search your code for it. The fix is checking event.source against a frame reference you created, because origin can't tell opaque frames apart.
The combination that disables your own sandbox
While I'm confessing, here's the other classic. The MDN iframe reference warns that combining allow-scripts and allow-same-origin on a frame whose content is served from your own origin lets that content reach into the parent and remove the sandbox attribute entirely. It's a one-line escape. People add both flags because the embedded app breaks without same-origin access to local storage, the app starts working, and the ticket closes.
If you host user-supplied HTML, serve it from a different registrable domain, not just a different subdomain, and keep allow-same-origin off. Separate origin for untrusted content is the boring, old, reliable answer. Google, GitHub and others have used the same trick for years for exactly this reason.
My rule of thumb now:
<iframe
src="https://preview.example-usercontent.net/abc123"
sandbox="allow-scripts"
referrerpolicy="no-referrer"
loading="lazy"
></iframe>
Scripts allowed, nothing else, different site entirely. If the preview needs forms or popups I add those flags one at a time and write down why in the commit message.
The allowlist entry nobody reviews
The detail in the story that bothers me most is the YouTube entry in the CSP. I'd have waved it through too. Embedding videos is a normal product need, and YouTube is not a hostile domain. But a script-src allowlist trusts everything that domain will serve, including endpoints that reflect input. Large shared domains tend to have many such endpoints.
If you want a stricter setup, nonce-based CSP with strict-dynamic avoids the host allowlist problem altogether. I wrote about the nonce route, and the one inline style attribute I couldn't lock down, in my React security post. It isn't free. Nonces mean server-rendered headers and some careful plumbing. But a host allowlist has a failure mode you can't see from the policy text, and a nonce doesn't.
I also want to say something about the bounty. Simon points out that the reported $5,000 payout seemed low for a bug that could expose a signed-in user's private data. I agree with him, with the caveat that I don't know the programme's rules or how OpenAI scored it. The takeaway for me is practical: a payout doesn't tell you how much the bug would have cost if someone less friendly found it first.
Things to do this week
Open your codebase and search for addEventListener('message' and onmessage. Every hit is a trust boundary. For each one, answer three questions: who can send to this window, what happens if they lie about the payload shape, and does any value end up in innerHTML, outerHTML, insertAdjacentHTML or document.write?
Then search for sandbox= and look for any frame carrying both allow-scripts and allow-same-origin. If the content in that frame isn't entirely yours, you have the same shape of problem the researchers exploited. I do this kind of review for clients, and you can see what I work on.
Do the grep before lunch tomorrow. Fix the worst innerHTML first, because that single change would have broken the chain described above.
---
Originally published at abrarqasim.com. I write there about React, PHP, Rust, Go and the AI tooling around them.
The Secure Code Review Challenge — Solution #8: Employee Directory (One Setting Away from Admin Takeover)
Employee Directory does almost everything right. Invitation codes are random, hashed and single-use. JWTs have a pinned algorithm. Every admin route checks the role, and the role is re-read from the database on every request. Salaries are masked, profile updates go through allowlists, forms carry CSRF tokens and NoSQL operators are stripped from every request.
It still lets any employee run code in the admin's browser. All it takes is a <script> tag in their profile Name and one line in the template config: autoescape: false.
This post walks through the full review: how the app works, where user input goes, which defenses hold, which one is missing, a working exploit, and the fix. Every code snippet you need is included, so you don't have to open the repo to follow along. Each snippet links to its exact lines on GitHub if you want more context.
🎥 Prefer video? The full walkthrough is on YouTube:
{% embed youtu.be %}
---
What is the Secure Code Review Challenge?
It's a free, biweekly series. Each challenge is a complete, realistic application with its own database, backend and UI, and one vulnerability planted on purpose, based on a real-world CVE or writeup. You review it the way you'd review a real codebase, then compare your reasoning with the published solution.
Every solution follows the same 7-step methodology:
1. 🗺️ Understand the application: architecture and main user stories
2. 🚪 Identify entry points: where user input comes in
3. 🎯 Identify dangerous sinks: where input could change behavior (queries, file paths, HTML, …)
4. 🧩 Build a threat model: business-logic risks (authN/authZ) and source-to-sink risks (injection)
5. 🔍 Review mitigations: which threats are actually closed, and which aren't
6. 🧪 Exploit: prove the unmitigated one is real
7. 🛠️ Fix: a primary fix plus defense-in-depth
📌 Repo: mohamed-osama-aboelkheir/the-secure-code-review-challenge
📌 Challenge #8 source: github.com/mohamed-osama-aboelkheir/the-secure-co…
> ⚠️ Spoiler warning: if you want to try Employee Directory yourself first, stop here and come back later.
---
1. 🗺️ Understanding the application
Employee Directory is an internal company phonebook. Employees look each other up, and admins manage the records.
- Backend: Express.js (Node)
- Frontend: server-rendered HTML pages, built with the Nunjucks template engine
- Database: MongoDB, through Mongoose
- Auth: a JWT, returned as a bearer token by the API and kept in an httpOnly cookie by the web UI
- Roles: employee and admin
The README sets out the rules the whole design protects:
> "Employees browse everyone's public fields, see salary only on their own record, and can edit the public fields of their own record — never salary. Admins see and edit every field of every record (including salary), delete records, invite new hires, and promote other users to admin."
So there are two things worth stealing: the salary field, and the admin role that unlocks it.
A good way to understand a codebase is to read it as a set of stories. We'll start with what happens when the app starts, then follow the main use cases: an admin invites a new hire, the new hire registers, they edit their profile, and everyone browses the directory.
🚀 Story 1: what happens when the app starts
docker-compose.yml starts two containers: a stock mongo:7 and the app, which gets its secrets from .env:
app:
build: .
container_name: directory_app
ports:
- "3000:3000"
environment:
MONGODB_URI: mongodb://mongo:27017/employee_directory
env_file:
- .env
docker-compose.yml#L17-L30
The Dockerfile installs production dependencies and runs node app.js as an unprivileged user. In app.js, the server refuses to start without a JWT secret:
if (!process.env.JWT_SECRET) {
console.error('JWT_SECRET is required');
process.exit(1);
}
app.js#L20-L23
Next it configures the template engine. Every page you see in the browser is a Nunjucks template in views/, and the engine fills in values like {{ emp.name }}. Read this config carefully. It matters later:
nunjucks.configure(viewsPath, {
autoescape: false,
express: app,
noCache: process.env.NODE_ENV !== 'production'
});
app.js#L56-L60
Then it registers the global middleware. Two of them are security controls we'll come back to: mongoSanitize and a CSRF check.
app.use(express.json());
app.use(express.urlencoded({ extended: true }));
app.use(cookieParser());
app.use(mongoSanitize()); // strips keys containing $ or . → blocks NoSQL operator injection
app.use(optionalAuth); // sets req.user if a valid token is present
...
app.use(csrfMiddleware); // csurf on every non-exempt state-changing request
app.js#L63-L79
Finally it mounts four routers: the server-rendered pages at /, plus the auth, employees and admin JSON APIs:
app.use('/', pagesRoutes);
app.use('/api/auth', authRoutes);
app.use('/api/employees', employeesRoutes);
app.use('/api/admin', adminRoutes);
app.js#L85-L88
On first boot, with an empty database, the app creates a bootstrap admin with a random password and prints it to the logs:
const password = crypto.randomBytes(12).toString('base64url');
const passwordHash = await bcrypt.hash(password, 10);
const user = await store.createUser('admin@localhost', passwordHash, 'admin', null);
src/bootstrap.js#L9-L11
🔄 Story 2: the admin invites a new hire
The admin fills in an email on /admin and clicks Create invitation. The API version of that route sits behind two middlewares for the whole router:
router.use(requireAuth, requireAdmin);
src/routes/admin.js#L11
The store voids any older codes for that email, generates 16 random bytes and stores only their bcrypt hash. The plaintext is returned once and never saved:
async createInvitation(email) {
const normalized = email.toLowerCase().trim();
await Invitation.updateMany(
{ email: normalized, used: false },
{ $set: { used: true } }
);
const plaintext = crypto.randomBytes(16).toString('hex');
const codeHash = await bcrypt.hash(plaintext, 10);
const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000);
await Invitation.create({ email: normalized, codeHash, expiresAt, used: false });
return { email: normalized, invitationCode: plaintext, expiresAt };
}
src/store/directoryStore.js#L224-L240
🔄 Story 3: the new hire registers and logs in
POST /api/auth/register takes email, password and invitationCode. It rejects duplicate emails, checks and consumes the invitation, then creates the user with a role that is hard-coded to employee:
const okInvite = await db.validateAndConsumeInvitation(email, invitationCode);
if (!okInvite) { ... return res.status(400).json({ error: 'Invalid or expired invitation' }); }
const passwordHash = await bcrypt.hash(password, 10);
const user = await db.createUser(email, passwordHash, 'employee', null);
src/routes/auth.js#L66-L76
The invitation check treats the code like a password. It finds unused, unexpired invitations for that email, compares with bcrypt, and marks the match as used:
const invitations = await Invitation.find({
email: normalized,
used: false,
expiresAt: { $gt: new Date() }
}).sort({ expiresAt: -1 });
for (const inv of invitations) {
const match = await bcrypt.compare(plaintextCode, inv.codeHash);
if (match) {
inv.used = true;
await inv.save();
return true;
}
}
return false;
src/store/directoryStore.js#L252-L269
Login compares the bcrypt hash and signs an HS256 JWT. The web form also stores it in an httpOnly, SameSite=Lax cookie, so JavaScript on the page can't read it:
const cookieOptions = {
httpOnly: true,
maxAge: 24 * 60 * 60 * 1000,
path: '/',
sameSite: 'lax'
};
src/routes/auth.js#L21-L26
On every later request, requireAuth verifies the token with the algorithm pinned, then reloads the user from the database. So req.user.role is always the current role in MongoDB, not whatever the token says:
const decoded = jwt.verify(token, secret, { algorithms: ['HS256'] });
const user = await db.findUserById(decoded.userId);
if (!user) {
return res.status(401).json({ error: 'User not found' });
}
req.user = {
id: user.id,
email: user.email,
role: user.role,
employeeId: user.employeeId
};
src/middleware/auth.js#L36-L47
🔄 Story 4: an employee updates their own profile
The My profile form posts to POST /profile (the API equivalent is PATCH /api/employees/me). The handler finds the caller's record from req.user.id, never from an ID in the request, and copies only an allowlist of public fields:
const emp = await db.findEmployeeByUserId(req.user.id);
...
const allowed = ['name', 'title', 'manager', 'deskNumber', 'location', 'phone'];
const patch = {};
for (const key of allowed) {
if (Object.prototype.hasOwnProperty.call(req.body, key)) {
patch[key] = req.body[key];
}
}
await db.updateEmployeeById(emp.id, patch);
src/routes/pages.js#L64-L75
The allowlist controls which fields you can write. Nothing checks what you write into them: no length limit, no character rules. Whatever you type into Name is saved as-is.
🔄 Story 5: everyone browses the directory
GET / asks the store for the directory, filtered for the viewer. Admins and the record's owner get the full row. Everyone else gets a masked copy without salary or email:
async getDirectoryForViewer(viewerUserId, viewerRole) {
const employees = await this.listEmployees();
const viewerIdStr = viewerUserId ? String(viewerUserId) : null;
return employees.map((emp) => {
if (viewerRole === 'admin') {
return { ...emp };
}
if (emp.userId && emp.userId === viewerIdStr) {
return { ...emp };
}
return this.maskEmployee(emp);
});
}
src/store/directoryStore.js#L210-L222
The page then prints every field into a table cell:
{% for emp in employees %}
<tr>
<td>{{ emp.name }}</td>
<td>{{ emp.title }}</td>
<td>{{ emp.manager }}</td>
<td>{{ emp.deskNumber }}</td>
<td>{{ emp.location }}</td>
<td>{{ emp.phone }}</td>
views/directory.html#L19-L26
The admin page (GET /admin) goes further. It lists every employee as an editable form, with each stored value inside an <input value="...">, next to the Make admin buttons:
<label>Name <input type="text" name="name" value="{{ emp.name }}"></label>
...
<form method="post" action="/admin/promote/{{ u.id }}" style="display:inline;">
<input type="hidden" name="_csrf" value="{{ csrfToken() }}">
<button type="submit">Make admin</button>
</form>
views/admin.html#L45 · views/admin.html#L70-L73
🧠 The key observation from step 1
Put stories 4 and 5 side by side:
any employee ──writes──▶ name, title, location, … ──stored as typed──▶ MongoDB
│
admin's browser ◀──renders {{ emp.name }}── directory.html / admin.html ◀─┘
(the page that holds the "Make admin" buttons)
Text written by the least privileged user is rendered in the page of the most privileged user. That's a trust boundary. Everything depends on the template engine turning that text into harmless characters on the way out.
---
2. 🚪 Entry points
- POST /api/auth/register: no auth (needs a valid invitation), takes email, password, invitationCode
- POST /api/auth/login: no auth, takes email, password
- POST /api/auth/logout: no auth
- GET /api/employees: any user, no input
- PATCH /api/employees/me: any user, takes name, title, manager, deskNumber, location, phone
- POST /api/admin/invitations: admin, takes email
- GET /api/admin/invitations, GET /api/admin/employees, GET /api/admin/users: admin, no input
- PATCH /api/admin/employees/:id: admin, takes :id and the whole body
- DELETE /api/admin/employees/:id: admin, takes :id
- PATCH /api/admin/users/:id: admin, takes :id, role
- GET /login, GET /register: no auth, take ?error=
- GET /: any user, renders stored data
- GET /profile, POST /profile: any user (+ CSRF on the POST), the public fields
- GET /admin: admin, renders stored data
- POST /admin/employee/:id, POST /admin/promote/:id, POST /admin/invite: admin + CSRF, :id and form fields
- GET /api/csrf-token, GET /health: no auth
3. 🎯 Dangerous sinks
- S1: HTML rendering with Nunjucks. The templates print stored employee fields and the ?error= query parameter. If user input can become markup, you get XSS (CWE-79).
- S2: MongoDB queries and updates built from input → NoSQL operator injection. It comes in two flavors:
- S2a, query filters: email at login and register, invitationCode, :id. An operator like {"$ne": null} would change which documents match.
- S2b, the update document: updateEmployeeById(id, patch). An operator like $unset or $rename would change what the update does.
- S3: redirects. res.redirect(...) is used all over auth.js and pages.js → open redirect.
---
4 & 5. 🧩 Threat model & 🔍 mitigation review
🔓 Business logic
Authentication ✅. The app won't start without JWT_SECRET. jwt.verify pins HS256, which blocks alg: none and algorithm confusion (jsonwebtoken docs). Tokens expire after 24h, and the user is reloaded from the database on every request. Every admin API route has requireAuth, and every page that needs a user has requireUser.
const decoded = jwt.verify(token, secret, { algorithms: ['HS256'] });
const user = await db.findUserById(decoded.userId);
src/middleware/auth.js#L36-L37 · secret required, app.js#L20-L23 · 24h expiry, auth.js#L12-L19
requireUser, used by the web pages, only lets a request through once optionalAuth has verified the token and set req.user:
function requireUser(req, res, next) {
if (!req.user) {
return res.redirect('/login');
}
next();
}
src/routes/pages.js#L10-L15
Invitation-only registration ✅. Can someone register without an invitation, reuse one, or use someone else's? No. Codes are 128-bit random and only their bcrypt hash is stored, so even a database leak doesn't reveal a usable code. Each code is tied to one email and expires after 7 days, and creating a new one voids the old ones:
await Invitation.updateMany(
{ email: normalized, used: false },
{ $set: { used: true } }
);
const plaintext = crypto.randomBytes(16).toString('hex');
const codeHash = await bcrypt.hash(plaintext, 10);
const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000);
src/store/directoryStore.js#L226-L232
At registration, only an unused, unexpired code for that email is accepted, and the match is marked used so it works once:
const invitations = await Invitation.find({
email: normalized,
used: false,
expiresAt: { $gt: new Date() }
}).sort({ expiresAt: -1 });
...
inv.used = true;
await inv.save();
src/store/directoryStore.js#L254-L264
And the new account's role is hard-coded, so you can't register yourself as an admin. A second registration with the same email is blocked by the pre-checks and by the unique index on email:
const user = await db.createUser(email, passwordHash, 'employee', null);
src/routes/auth.js#L76 · duplicate checks, auth.js#L50-L64 · unique index, directoryStore.js#L6
Vertical privilege escalation (employee → admin) ✅. The admin API router applies both middlewares to every route, and requireAdmin checks the role that requireAuth just loaded from the database:
router.use(requireAuth, requireAdmin);
src/routes/admin.js#L11
const requireAdmin = async (req, res, next) => {
if (!req.user || req.user.role !== 'admin') {
return res.status(403).json({ error: 'Admin access required' });
}
next();
};
src/middleware/auth.js#L57-L62
The /admin* pages don't use that router, so each handler checks the role itself (pages.js#L85, #L106, #L128, #L140):
router.post('/admin/promote/:id', requireUser, async (req, res, next) => {
try {
if (req.user.role !== 'admin') {
return res.status(403).send('Forbidden');
}
await db.updateUserRole(req.params.id, 'admin');
res.redirect('/admin');
src/routes/pages.js#L126-L132
Keep that last detail in mind: the check is "is this request from an admin?", and it is.
Horizontal access to salaries ✅. getDirectoryForViewer (src/store/directoryStore.js#L210-L222, shown in story 5) returns maskEmployee for other people's rows. It builds a new object from the public fields only, so salary and email can't leak by accident:
maskEmployee(emp) {
return {
id: emp.id,
name: emp.name,
title: emp.title,
manager: emp.manager,
deskNumber: emp.deskNumber,
location: emp.location,
phone: emp.phone
};
}
src/store/directoryStore.js#L198-L208
The self-edit routes look up the record from req.user.id (src/routes/pages.js#L64, src/routes/employees.js#L30), never from an ID in the request, and the API rejects salary outright:
if (Object.prototype.hasOwnProperty.call(req.body, 'salary')) {
return res.status(403).json({ error: 'Cannot update salary through this endpoint' });
}
src/routes/employees.js#L35-L37
CSRF ✅. csurf checks a _csrf token on every state-changing request that isn't exempt. Requests with an Authorization: Bearer header skip it, which is safe: a page on another site can't make the browser add that header. Removing the _csrf field from an invite request in Burp Repeater gets Invalid or missing CSRF token. The cookie is also SameSite=Lax.
function csrfMiddleware(req, res, next) {
const auth = req.headers.authorization;
const hasBearer = auth && String(auth).startsWith('Bearer ');
if (hasBearer && !['GET', 'HEAD', 'OPTIONS'].includes(req.method)) {
return next();
}
if (req.method === 'POST' && CSRF_EXEMPT_AUTH_POST.has(req.path)) {
return next();
}
return csrfProtection(req, res, next);
}
app.js#L36-L46
One caveat for later: a CSRF token only stops requests from another origin. A script running on this origin can read the token like any form on the page does.
Mass assignment ✅. Can an employee write fields they shouldn't, like salary, email or userId? No. There are two allowlists. The employee routes copy only the six public fields (src/routes/pages.js#L68-L74, src/routes/employees.js#L39-L45, shown in story 4). The store then copies only its own fixed list of keys, so unknown fields like role or _id never reach MongoDB:
const allowed = [
'name', 'title', 'manager', 'deskNumber', 'location', 'phone',
'salary', 'userId', 'email'
];
src/store/directoryStore.js#L160-L170
salary, userId and email are on the store's list, but only the admin API passes the raw body to it, and that route sits behind requireAdmin:
router.patch('/employees/:id', async (req, res) => {
try {
const updated = await db.updateEmployeeById(req.params.id, req.body);
src/routes/admin.js#L47-L49
This matches the OWASP Mass Assignment cheat sheet's advice to allowlist the bindable fields.
💉 Source-to-sink
S2a, NoSQL injection in filters ✅. The classic attack sends an object instead of a string, for example {"email": {"$ne": null}}. express-mongo-sanitize strips $-prefixed and dotted keys from the body, query and params before any handler runs:
app.use(mongoSanitize());
app.js#L66
On top of that, findUserByEmail calls email.toLowerCase(), which throws on an object, and findUserById goes through Mongoose, which casts the ID to an ObjectId and rejects anything else:
async findUserByEmail(email) {
const doc = await User.findOne({ email: email.toLowerCase().trim() });
return toUserDto(doc);
}
async findUserById(id) {
const doc = await User.findById(id);
return toUserDto(doc);
}
src/store/directoryStore.js#L86-L94
S2b, NoSQL injection in updates ✅. updateEmployeeById builds a brand-new update object from a fixed list of keys, so a top-level $unset or $rename is never copied in, and mongo-sanitize strips nested $ keys too:
const update = {};
for (const key of allowed) {
if (Object.prototype.hasOwnProperty.call(patch, key)) {
...
update[key] = patch[key];
}
}
const doc = await Employee.findByIdAndUpdate(id, update, { new: true, runValidators: true });
src/store/directoryStore.js#L171-L186
S3, open redirect ✅. Every res.redirect target is a hard-coded relative path like /login or /profile?saved=1. User data only goes into the query string, and only after encodeURIComponent, so it can't change where the browser goes:
return res.redirect('/register?error=' + encodeURIComponent('All fields are required'));
...
res.redirect('/profile?saved=1');
src/routes/auth.js#L45 · src/routes/pages.js#L76
S1, XSS: this is where it breaks ❌. Go back to the template config from story 1:
nunjucks.configure(viewsPath, {
autoescape: false,
app.js#L56-L57
Autoescaping is the control that turns < into <, " into " and so on whenever a value is printed with {{ }}. It's exactly what the OWASP XSS Prevention cheat sheet calls output encoding. Nunjucks turns it on by default, so this line switches it off for every template at once.
Combine that with story 4 (Name is stored exactly as typed) and story 5 (Name is printed into the directory table and the admin page), and a profile field becomes HTML in the admin's browser.
> 💡 Reviewer's lesson: when you see a template engine, a UI framework or a markdown renderer, check its escaping setting before you read a single template. One global flag can undo every {{ value }} in the app, and it's a one-line change that's easy to miss in review.
---
6. 🧪 Exploitation
Why a plain <script> works
Where the value lands decides what the payload has to look like:
- The Directory, <td>{{ emp.name }}</td>: the value is element text, so a plain <script>…</script> becomes a real script tag. No breakout is needed.
- The admin page, <input value="{{ emp.name }}">: the value is inside a quoted attribute, so a payload needs a leading "> to close the attribute and the tag first.
- Your own profile page: the same attribute context, which is why the payload doesn't fire while you're typing it.
PoC
1. The employee puts a script in their Name. On My profile, they set Name to test2<script>alert('XSS')</script> and click Save. The server stores it as typed.
!The employee stores a script tag in the Name field of their profile
2. The admin logs in and opens the Directory. Admins land on /admin after login (top right: admin@localhost). From there they click Directory, an ordinary part of their day.
!The admin is logged in on /admin and clicks Directory
3. The script runs in the admin's browser. The alert comes from localhost:3000, the app's own origin, so the attacker's code is running inside the admin's session.
!An alert box saying XSS pops up on localhost:3000 for the admin
What a real payload can do
An alert is only the proof. A real payload can do anything the admin's page can do:
- Read the admin's CSRF token from GET /api/csrf-token, or from the hidden _csrf fields on /admin.
- Send admin requests as the admin. The session cookie is httpOnly, so the script can't read it, but it doesn't need to: the browser attaches it to every request the script sends.
- Promote the attacker. With the token and the cookie, a POST /admin/promote/<attacker-id> passes requireUser, the role check and the CSRF check, because it really does come from the admin's browser.
- Upgrade the attacker's existing session. requireAuth reloads the role from the database, so the attacker's own token becomes an admin token immediately.
Impact: any employee becomes an admin the next time an admin opens the Directory. From there they can read and change every salary, delete records, create invitations and promote others. The same payload also runs for every employee who opens the directory.
Classification: Stored Cross-Site Scripting, CWE-79, caused by disabled output encoding, CWE-116. It turns into privilege escalation because the victim is an admin.
---
7. 🛠️ The fix
Primary fix: turn output encoding back on. Delete the line or set it to true:
nunjucks.configure(viewsPath, {
- autoescape: false,
+ autoescape: true,
express: app,
noCache: process.env.NODE_ENV !== 'production'
});
Here is the real admin.html rendering the same kind of payload, before and after:
autoescape:false -> <input type="text" name="location" value=""><script>alert(1)</script>">
autoescape:true -> <input type="text" name="location" value=""><script>alert(1)</script>">
Nunjucks' escaping encodes & " ' < >, which covers both places this app prints values: element text and double-quoted attributes. None of the templates intentionally output HTML, so nothing needs | safe.
Defense-in-depth (do these too):
1. Add a Content Security Policy. None of the templates use inline <script>, so a policy like script-src 'self'; object-src 'none'; base-uri 'none' (for example with helmet) blocks this payload without breaking the UI.
2. Validate profile fields when they come in. Enforce lengths and per-field formats (phone, desk number), and reject < and > in plain-text fields. This is a safety net, not the fix: output encoding is still required. If you ever need rich text, use an allowlist sanitizer like DOMPurify instead of | safe.
3. Make privilege changes harder to ride. Require re-authentication or a confirmation step for Make admin, and audit-log role changes.
4. Guard the config in CI. No public Semgrep ruleset flags autoescape: false for Nunjucks, but a custom rule takes a few lines:
rules:
- id: nunjucks-autoescape-disabled
languages: [javascript, typescript]
severity: ERROR
message: Nunjucks is configured with autoescape disabled, so every {{ value }} is written into the HTML unencoded.
pattern-either:
- pattern: "$NUNJUCKS.configure(..., {..., autoescape: false, ...})"
- pattern: "new $NUNJUCKS.Environment(..., {..., autoescape: false, ...})"
nunjucks-autoescape-disabled.yaml
It flags the root cause directly:
$ semgrep scan --metrics=off \
--config solutions/008-employee-directory/nunjucks-autoescape-disabled.yaml \
challenges/008-employee-directory
challenges/008-employee-directory/app.js
❯❯❱ solutions.008-employee-directory.nunjucks-autoescape-disabled
56┆ nunjucks.configure(viewsPath, {
57┆ autoescape: false,
58┆ express: app,
59┆ noCache: process.env.NODE_ENV !== 'production'
60┆ });
The rule points at the config, not at each {{ emp.name }}, which is the right place: one flag turns off encoding for every template. A taint rule wouldn't find this anyway, because the payload is saved to MongoDB in one request and rendered in another.
---
🌍 Real-world examples
- Stored XSS to Privilege Escalation to Admin Takeover to Data Breach — Ahmad Halabi (Aug 2025). A bug bounty write-up on a HackerOne financial program. An employee could only see the loan they were invited to, but the loan's purchase description was rendered without encoding. The researcher stored a <script src=…> payload there and invited the manager to review the loan. When the manager opened it, the script read the manager's CSRF token and replayed the role-change request, making the employee a manager. Almost exactly this challenge's chain.
- From a Sanitized Name Field to One-Click Account Takeover — zere.es (2026). The server stripped HTML from a name field, but the front-end decoded HTML entities and wrote the value with innerHTML, so entity-encoded markup survived and ran. The author chained it with login CSRF and OAuth-code theft for one-click takeover. A different sink with the same lesson: filtering input doesn't help when the output isn't encoded for its context.
📚 More resources
- CWE-79: Cross-site Scripting and CWE-116: Improper Encoding or Escaping of Output
- OWASP Cross Site Scripting Prevention Cheat Sheet
- Nunjucks: Autoescaping and configure options
- PortSwigger Web Security Academy: Stored XSS
- PortSwigger lab: Exploiting XSS to bypass CSRF defenses
- MDN: Content Security Policy
---
🎯 Takeaway
Employee Directory's server-side authorization is sound: role checks, salary masking, allowlists and CSRF tokens all hold, and httpOnly keeps the session cookie out of reach. One template setting makes them irrelevant. They all still run, but once attacker script runs on your origin in an admin's browser, every check sees a legitimate admin request.
Whenever something renders in the browser, whether it's React, Angular, a template engine or a markdown renderer, check that user input goes through HTML escaping on the way out. Keep autoescape on, and treat any field a low-privileged user writes and a high-privileged user reads as a trust boundary.
---
🔔 What's next
Challenge #9: ProjectHub is live now. Can you find the bug before the solution drops?
👉 github.com/mohamed-osama-aboelkheir/the-secure-co…
New challenges land every two weeks, and each drop includes the previous challenge's solution. Watch the repo (Watch → Custom → Releases) to get notified, and subscribe to AppSec Untangled on YouTube for the video walkthroughs.
If you found the bug yourself, or took a different route to it, tell me in the comments 👇
Mentions
iframe Sandbox Lessons From the ChatGPT Account Takeover I have a confession about iframes. For years I treated the sandbox attribute like a seatbelt: I clicked it in, felt safe, and never once asked what the car was doing. Then I read Simon Willison's link post about how researchers took over ChatGPT accounts and realised the seatbelt was fine. The problem was a door nobody had locked, three feet away. Short version for the impatient: the sandbox held. The attack worked because the parent page had a message handler that wrote whatever it received into the DOM with innerHTML, and it never asked who was talking. If your app embeds untrusted HTML and also listens for postMessage, that handler is the thing to audit this week, not the iframe attribute. I'll walk through the chain as Simon summarised it, then show the handler pattern I now use in my own code. I'm working from his write-up and not from the original researchers' report, so treat my account of the details as a summary and go read the sources yourself. What the attack chain actually did ChatGPT lets you preview interactive HTML apps in a sandboxed iframe. That's the same idea as the artifacts pattern you may have seen elsewhere. The point of the sandbox is that the HTML inside is untrusted. It might be written by a stranger, shared through a link, and opened by you while signed in. According to Simon's summary, the researchers at Parameter Labs found a chain of small problems. The parent page used the MathLive library for graph support, and MathLive ships its own virtual keyboard. That keyboard can be triggered in the parent page through postMessage(). The receiving code used innerHTML, so anything able to send a message could push HTML into the parent. Nothing stopped a sandboxed preview frame from sending one. Inserting HTML is not the same as running script. innerHTML won't execute a <script> element you hand it. But it will happily create an <iframe>, and a new iframe created by the parent has no sandbox unless the parent adds one. So the attacker got an unsandboxed frame inside the page. Then came the content security policy. It blocked inline scripts and only allowed scripts from a short list of domains. One of those domains was YouTube, and the YouTube oEmbed endpoint could be coaxed into reflecting JavaScript back in a form that ran. With script running on the ChatGPT origin, fetching private data from the APIs and sending it out through an image request was, in Simon's word, trivial. Share a link, wait for a signed-in person to open it, collect their data. I find the shape of this more useful than the specific bug. Four separate decisions, each defensible in a code review on its own, added up to account takeover. A message handler that trusts its sender. A DOM write that accepts markup. A frame that inherits no restrictions. An allowlist entry for a big, friendly domain. Remove any single one and the chain breaks. Why the sandbox attribute wasn't the problem The sandbox attribute did its job. A sandboxed frame without allow-same-origin gets an opaque origin, so it can't read the parent's cookies or storage, and it can't touch the parent's DOM directly. What it can still do, if you let it run scripts, is call window.parent.postMessage(). That channel is deliberate. It's the supported way for a restricted frame to talk to the page around it. So the real question is what your page does when a message arrives. Think of that handler as a public API endpoint, because functionally it is one. Any frame that can reach your window can call it, including frames you never meant to host. Here's the pattern that worries me: // Before: trusts every message from every frame window.addEventListener('message', (event) => { const { type, html } = event.data; if (type === 'show-keyboard') { document.getElementById('kb').innerHTML = html; } }); Nothing here checks event.origin. Nothing checks event.source. The payload shape is whatever the sender says it is. And the innerHTML write means the sender controls markup, which is the exact step the researchers needed. A handler I would actually ship The MDN documentation for postMessage is blunt about this: always verify the sender's identity using the origin and possibly source properties, and be wary of what you do with the data. Here's my current version, rewritten after reading the ChatGPT write-up: // After: known sender, known shape, no markup from outside const KEYBOARD_ORIGIN = 'https://app.example.com'; const keyboardFrame = document.getElementById('keyboard-frame'); window.addEventListener('message', (event) => { if (event.origin !== KEYBOARD_ORIGIN) return; if (event.source !== keyboardFrame.contentWindow) return; const data = event.data; if (!data || data.type !== 'show-keyboard') return; if (typeof data.layout !== 'string') return; const layouts = { numeric: 'numeric', text: 'text' }; const layout = layouts[data.layout]; if (!layout) return; document.getElementById('kb').dataset.layout = layout; }); Four things changed. The handler compares the origin against one exact string, not a prefix and not endsWith. It also compares event.source to the specific frame I expect, because two different frames can share an origin. The payload is validated against a closed set of values. And nothing from the message ever becomes markup. It becomes a data- attribute that my own CSS reads. That last change is the one that matters most. If a handler needs to render something the sender supplied, use textContent, or build elements with createElement. If it genuinely has to accept markup, run it through a sanitizer such as DOMPurify and then think hard about whether you really needed markup. One more trap with sandboxed frames: their origin is the string "null". If you ever write if (event.origin === 'null') to accept messages from a sandboxed preview, you've just accepted messages from every sandboxed frame on the internet. It's an easy line to write when a preview pane refuses to load, so search your code for it. The fix is checking event.source against a frame reference you created, because origin can't tell opaque frames apart. The combination that disables your own sandbox While I'm confessing, here's the other classic. The MDN iframe reference warns that combining allow-scripts and allow-same-origin on a frame whose content is served from your own origin lets that content reach into the parent and remove the sandbox attribute entirely. It's a one-line escape. People add both flags because the embedded app breaks without same-origin access to local storage, the app starts working, and the ticket closes. If you host user-supplied HTML, serve it from a different registrable domain, not just a different subdomain, and keep allow-same-origin off. Separate origin for untrusted content is the boring, old, reliable answer. Google, GitHub and others have used the same trick for years for exactly this reason. My rule of thumb now: <iframe src="https://preview.example-usercontent.net/abc123" sandbox="allow-scripts" referrerpolicy="no-referrer" loading="lazy" ></iframe> Scripts allowed, nothing else, different site entirely. If the preview needs forms or popups I add those flags one at a time and write down why in the commit message. The allowlist entry nobody reviews The detail in the story that bothers me most is the YouTube entry in the CSP. I'd have waved it through too. Embedding videos is a normal product need, and YouTube is not a hostile domain. But a script-src allowlist trusts everything that domain will serve, including endpoints that reflect input. Large shared domains tend to have many such endpoints. If you want a stricter setup, nonce-based CSP with strict-dynamic avoids the host allowlist problem altogether. I wrote about the nonce route, and the one inline style attribute I couldn't lock down, in my React security post. It isn't free. Nonces mean server-rendered headers and some careful plumbing. But a host allowlist has a failure mode you can't see from the policy text, and a nonce doesn't. I also want to say something about the bounty. Simon points out that the reported $5,000 payout seemed low for a bug that could expose a signed-in user's private data. I agree with him, with the caveat that I don't know the programme's rules or how OpenAI scored it. The takeaway for me is practical: a payout doesn't tell you how much the bug would have cost if someone less friendly found it first. Things to do this week Open your codebase and search for addEventListener('message' and onmessage. Every hit is a trust boundary. For each one, answer three questions: who can send to this window, what happens if they lie about the payload shape, and does any value end up in innerHTML, outerHTML, insertAdjacentHTML or document.write? Then search for sandbox= and look for any frame carrying both allow-scripts and allow-same-origin. If the content in that frame isn't entirely yours, you have the same shape of problem the researchers exploited. I do this kind of review for clients, and you can see what I work on. Do the grep before lunch tomorrow. Fix the worst innerHTML first, because that single change would have broken the chain described above. --- Originally published at abrarqasim.com. I write there about React, PHP, Rust, Go and the AI tooling around them.
After the DOMPurify Patch: Hardening Rich Content Rendering Against CVE-2026-65898 After the DOMPurify Patch: Hardening Rich Content Rendering Against CVE-2026-65898 +Patching DOMPurify to 3.4.11 or later closes the known shared-allowlist defect. It does not by itself prove that every rich-content workflow is safe. Applications can still share sanitizer state across trust domains, grant attributes too broadly, or rely on browser controls to compensate for unsafe output. CVE-2026-65898 is a useful point to review those assumptions. +## Vulnerability overview +DOMPurify through 3.4.10 can retain an attribute added by an uponSanitizeAttribute hook on the setConfig() path. The upstream advisory describes stored XSS when a trusted render pollutes the allowlist and a later untrusted render inherits the dangerous attribute. The patched upstream versions are 3.4.11 and later. +Splunk Enterprise fixed the bundled component in 9.4.16, 10.0.11, 10.2.8, and 10.4.4 or later. The advisory says the dashboard-studio package now uses DOMPurify 3.4.14. It does not report a real-world Splunk exploitation campaign. +## Separate sanitizer state by trust domain +The clean design uses a sanitizer instance with a policy that is appropriate to one trust domain. Administrative content, imported content, and ordinary user content may need different policies or no HTML at all. If they share an instance, the lifetime of a hook mutation becomes the lifetime of the shared policy. +Use per-call configuration when it fits the workflow. When a persistent policy is required, create separate instances and make the boundary explicit in code. Avoid a shared instance whose allowlist changes based on a marker that an attacker might influence. The marker should not be the only thing separating a trusted attribute from an untrusted one. +data.keepAttr = true can allow an attribute for the current element without writing into the shared allowlist, according to the upstream mitigation guidance. clearConfig() can reset state between trust domains, but relying on every caller to reset the same global instance is harder to verify than giving the domains separate instances. +## Keep the allowlist narrow +CVE-2026-65898 is most damaging when the polluted attribute is executable. Review the attributes that the application permits. Remove event handlers, URL-bearing attributes, and rich markup that the interface does not need. A sanitizer cannot protect a policy that intentionally accepts executable attributes. +Do not add a broad compatibility exception because a widget failed to render. Define the allowed tags and attributes for the feature, test the resulting HTML, and keep the exception local to the component that needs it. +## Make the browser defense independent +A Content Security Policy can reduce the effect of injected script. It does not fix sanitizer state. Deploy CSP as a separate control, monitor violation reports, and test the application with the production policy. Nonces and hashes should be managed so a legitimate widget update does not silently weaken the page. +Cookie attributes, token storage, and response authorization also shape the impact of stored XSS. Review whether sensitive browser APIs and administrative actions are available from the pages that render user-controlled content. This review is broader than the vendor patch and should include product owners. +## Establish a release gate +The deployment gate should verify the fixed Splunk release and the named DOMPurify asset. For custom applications, require a package version and a browser regression test. The test should display a trusted element that exercises the hook, then an untrusted element that should lose the same attribute. +Record the test result with the build hash and browser version. If the application does not expose the dynamic hook, record that finding and explain how the code was reviewed. NOT_APPLICABLE should be supported by evidence, not inferred from the vendor severity. +## Monitor the stored-content path +Monitor the application routes that render rich content and the sources that can create it. Capture authorization changes, unexpected modifications, and security events around the page. Preserve the raw stored value when investigating a report, because a rendered screenshot may omit the attribute that caused the issue. +Application owners should know which groups can create content, which groups can view it, and what the page can do in the browser. That information turns a generic XSS advisory into a concrete exposure decision. +## Exposure context +ZoomEye app="Splunk" returned 47,695 product-fingerprint matches on November 3, 2026. The CVE filter returned zero. The number supports asset discovery and patching scope. It does not prove a vulnerable release, a reachable hook, or an active compromise. +## Practical end state +Upgrade the platform, verify the bundled package, isolate sanitizer instances by trust domain, narrow the allowed attributes, and test stateful rendering behavior. Keep the policy and test evidence with the application release. The program should be able to explain why a rich-content path is safe after the dependency update, not only that the dependency version changed. +## References +- DOMPurify GHSA-cmwh-pvxp-8882 +- Splunk SVD-2026-1009 +- Red Hat CVE-2026-65898
The Secure Code Review Challenge — Solution #8: Employee Directory (One Setting Away from Admin Takeover) Employee Directory does almost everything right. Invitation codes are random, hashed and single-use. JWTs have a pinned algorithm. Every admin route checks the role, and the role is re-read from the database on every request. Salaries are masked, profile updates go through allowlists, forms carry CSRF tokens and NoSQL operators are stripped from every request. It still lets any employee run code in the admin's browser. All it takes is a <script> tag in their profile Name and one line in the template config: autoescape: false. This post walks through the full review: how the app works, where user input goes, which defenses hold, which one is missing, a working exploit, and the fix. Every code snippet you need is included, so you don't have to open the repo to follow along. Each snippet links to its exact lines on GitHub if you want more context. 🎥 Prefer video? The full walkthrough is on YouTube: {% embed youtu.be %} --- What is the Secure Code Review Challenge? It's a free, biweekly series. Each challenge is a complete, realistic application with its own database, backend and UI, and one vulnerability planted on purpose, based on a real-world CVE or writeup. You review it the way you'd review a real codebase, then compare your reasoning with the published solution. Every solution follows the same 7-step methodology: 1. 🗺️ Understand the application: architecture and main user stories 2. 🚪 Identify entry points: where user input comes in 3. 🎯 Identify dangerous sinks: where input could change behavior (queries, file paths, HTML, …) 4. 🧩 Build a threat model: business-logic risks (authN/authZ) and source-to-sink risks (injection) 5. 🔍 Review mitigations: which threats are actually closed, and which aren't 6. 🧪 Exploit: prove the unmitigated one is real 7. 🛠️ Fix: a primary fix plus defense-in-depth 📌 Repo: mohamed-osama-aboelkheir/the-secure-code-review-challenge 📌 Challenge #8 source: github.com/mohamed-osama-aboelkheir/the-secure-co… > ⚠️ Spoiler warning: if you want to try Employee Directory yourself first, stop here and come back later. --- 1. 🗺️ Understanding the application Employee Directory is an internal company phonebook. Employees look each other up, and admins manage the records. - Backend: Express.js (Node) - Frontend: server-rendered HTML pages, built with the Nunjucks template engine - Database: MongoDB, through Mongoose - Auth: a JWT, returned as a bearer token by the API and kept in an httpOnly cookie by the web UI - Roles: employee and admin The README sets out the rules the whole design protects: > "Employees browse everyone's public fields, see salary only on their own record, and can edit the public fields of their own record — never salary. Admins see and edit every field of every record (including salary), delete records, invite new hires, and promote other users to admin." So there are two things worth stealing: the salary field, and the admin role that unlocks it. A good way to understand a codebase is to read it as a set of stories. We'll start with what happens when the app starts, then follow the main use cases: an admin invites a new hire, the new hire registers, they edit their profile, and everyone browses the directory. 🚀 Story 1: what happens when the app starts docker-compose.yml starts two containers: a stock mongo:7 and the app, which gets its secrets from .env: app: build: . container_name: directory_app ports: - "3000:3000" environment: MONGODB_URI: mongodb://mongo:27017/employee_directory env_file: - .env docker-compose.yml#L17-L30 The Dockerfile installs production dependencies and runs node app.js as an unprivileged user. In app.js, the server refuses to start without a JWT secret: if (!process.env.JWT_SECRET) { console.error('JWT_SECRET is required'); process.exit(1); } app.js#L20-L23 Next it configures the template engine. Every page you see in the browser is a Nunjucks template in views/, and the engine fills in values like {{ emp.name }}. Read this config carefully. It matters later: nunjucks.configure(viewsPath, { autoescape: false, express: app, noCache: process.env.NODE_ENV !== 'production' }); app.js#L56-L60 Then it registers the global middleware. Two of them are security controls we'll come back to: mongoSanitize and a CSRF check. app.use(express.json()); app.use(express.urlencoded({ extended: true })); app.use(cookieParser()); app.use(mongoSanitize()); // strips keys containing $ or . → blocks NoSQL operator injection app.use(optionalAuth); // sets req.user if a valid token is present ... app.use(csrfMiddleware); // csurf on every non-exempt state-changing request app.js#L63-L79 Finally it mounts four routers: the server-rendered pages at /, plus the auth, employees and admin JSON APIs: app.use('/', pagesRoutes); app.use('/api/auth', authRoutes); app.use('/api/employees', employeesRoutes); app.use('/api/admin', adminRoutes); app.js#L85-L88 On first boot, with an empty database, the app creates a bootstrap admin with a random password and prints it to the logs: const password = crypto.randomBytes(12).toString('base64url'); const passwordHash = await bcrypt.hash(password, 10); const user = await store.createUser('admin@localhost', passwordHash, 'admin', null); src/bootstrap.js#L9-L11 🔄 Story 2: the admin invites a new hire The admin fills in an email on /admin and clicks Create invitation. The API version of that route sits behind two middlewares for the whole router: router.use(requireAuth, requireAdmin); src/routes/admin.js#L11 The store voids any older codes for that email, generates 16 random bytes and stores only their bcrypt hash. The plaintext is returned once and never saved: async createInvitation(email) { const normalized = email.toLowerCase().trim(); await Invitation.updateMany( { email: normalized, used: false }, { $set: { used: true } } ); const plaintext = crypto.randomBytes(16).toString('hex'); const codeHash = await bcrypt.hash(plaintext, 10); const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); await Invitation.create({ email: normalized, codeHash, expiresAt, used: false }); return { email: normalized, invitationCode: plaintext, expiresAt }; } src/store/directoryStore.js#L224-L240 🔄 Story 3: the new hire registers and logs in POST /api/auth/register takes email, password and invitationCode. It rejects duplicate emails, checks and consumes the invitation, then creates the user with a role that is hard-coded to employee: const okInvite = await db.validateAndConsumeInvitation(email, invitationCode); if (!okInvite) { ... return res.status(400).json({ error: 'Invalid or expired invitation' }); } const passwordHash = await bcrypt.hash(password, 10); const user = await db.createUser(email, passwordHash, 'employee', null); src/routes/auth.js#L66-L76 The invitation check treats the code like a password. It finds unused, unexpired invitations for that email, compares with bcrypt, and marks the match as used: const invitations = await Invitation.find({ email: normalized, used: false, expiresAt: { $gt: new Date() } }).sort({ expiresAt: -1 }); for (const inv of invitations) { const match = await bcrypt.compare(plaintextCode, inv.codeHash); if (match) { inv.used = true; await inv.save(); return true; } } return false; src/store/directoryStore.js#L252-L269 Login compares the bcrypt hash and signs an HS256 JWT. The web form also stores it in an httpOnly, SameSite=Lax cookie, so JavaScript on the page can't read it: const cookieOptions = { httpOnly: true, maxAge: 24 * 60 * 60 * 1000, path: '/', sameSite: 'lax' }; src/routes/auth.js#L21-L26 On every later request, requireAuth verifies the token with the algorithm pinned, then reloads the user from the database. So req.user.role is always the current role in MongoDB, not whatever the token says: const decoded = jwt.verify(token, secret, { algorithms: ['HS256'] }); const user = await db.findUserById(decoded.userId); if (!user) { return res.status(401).json({ error: 'User not found' }); } req.user = { id: user.id, email: user.email, role: user.role, employeeId: user.employeeId }; src/middleware/auth.js#L36-L47 🔄 Story 4: an employee updates their own profile The My profile form posts to POST /profile (the API equivalent is PATCH /api/employees/me). The handler finds the caller's record from req.user.id, never from an ID in the request, and copies only an allowlist of public fields: const emp = await db.findEmployeeByUserId(req.user.id); ... const allowed = ['name', 'title', 'manager', 'deskNumber', 'location', 'phone']; const patch = {}; for (const key of allowed) { if (Object.prototype.hasOwnProperty.call(req.body, key)) { patch[key] = req.body[key]; } } await db.updateEmployeeById(emp.id, patch); src/routes/pages.js#L64-L75 The allowlist controls which fields you can write. Nothing checks what you write into them: no length limit, no character rules. Whatever you type into Name is saved as-is. 🔄 Story 5: everyone browses the directory GET / asks the store for the directory, filtered for the viewer. Admins and the record's owner get the full row. Everyone else gets a masked copy without salary or email: async getDirectoryForViewer(viewerUserId, viewerRole) { const employees = await this.listEmployees(); const viewerIdStr = viewerUserId ? String(viewerUserId) : null; return employees.map((emp) => { if (viewerRole === 'admin') { return { ...emp }; } if (emp.userId && emp.userId === viewerIdStr) { return { ...emp }; } return this.maskEmployee(emp); }); } src/store/directoryStore.js#L210-L222 The page then prints every field into a table cell: {% for emp in employees %} <tr> <td>{{ emp.name }}</td> <td>{{ emp.title }}</td> <td>{{ emp.manager }}</td> <td>{{ emp.deskNumber }}</td> <td>{{ emp.location }}</td> <td>{{ emp.phone }}</td> views/directory.html#L19-L26 The admin page (GET /admin) goes further. It lists every employee as an editable form, with each stored value inside an <input value="...">, next to the Make admin buttons: <label>Name <input type="text" name="name" value="{{ emp.name }}"></label> ... <form method="post" action="/admin/promote/{{ u.id }}" style="display:inline;"> <input type="hidden" name="_csrf" value="{{ csrfToken() }}"> <button type="submit">Make admin</button> </form> views/admin.html#L45 · views/admin.html#L70-L73 🧠 The key observation from step 1 Put stories 4 and 5 side by side: any employee ──writes──▶ name, title, location, … ──stored as typed──▶ MongoDB │ admin's browser ◀──renders {{ emp.name }}── directory.html / admin.html ◀─┘ (the page that holds the "Make admin" buttons) Text written by the least privileged user is rendered in the page of the most privileged user. That's a trust boundary. Everything depends on the template engine turning that text into harmless characters on the way out. --- 2. 🚪 Entry points - POST /api/auth/register: no auth (needs a valid invitation), takes email, password, invitationCode - POST /api/auth/login: no auth, takes email, password - POST /api/auth/logout: no auth - GET /api/employees: any user, no input - PATCH /api/employees/me: any user, takes name, title, manager, deskNumber, location, phone - POST /api/admin/invitations: admin, takes email - GET /api/admin/invitations, GET /api/admin/employees, GET /api/admin/users: admin, no input - PATCH /api/admin/employees/:id: admin, takes :id and the whole body - DELETE /api/admin/employees/:id: admin, takes :id - PATCH /api/admin/users/:id: admin, takes :id, role - GET /login, GET /register: no auth, take ?error= - GET /: any user, renders stored data - GET /profile, POST /profile: any user (+ CSRF on the POST), the public fields - GET /admin: admin, renders stored data - POST /admin/employee/:id, POST /admin/promote/:id, POST /admin/invite: admin + CSRF, :id and form fields - GET /api/csrf-token, GET /health: no auth 3. 🎯 Dangerous sinks - S1: HTML rendering with Nunjucks. The templates print stored employee fields and the ?error= query parameter. If user input can become markup, you get XSS (CWE-79). - S2: MongoDB queries and updates built from input → NoSQL operator injection. It comes in two flavors: - S2a, query filters: email at login and register, invitationCode, :id. An operator like {"$ne": null} would change which documents match. - S2b, the update document: updateEmployeeById(id, patch). An operator like $unset or $rename would change what the update does. - S3: redirects. res.redirect(...) is used all over auth.js and pages.js → open redirect. --- 4 & 5. 🧩 Threat model & 🔍 mitigation review 🔓 Business logic Authentication ✅. The app won't start without JWT_SECRET. jwt.verify pins HS256, which blocks alg: none and algorithm confusion (jsonwebtoken docs). Tokens expire after 24h, and the user is reloaded from the database on every request. Every admin API route has requireAuth, and every page that needs a user has requireUser. const decoded = jwt.verify(token, secret, { algorithms: ['HS256'] }); const user = await db.findUserById(decoded.userId); src/middleware/auth.js#L36-L37 · secret required, app.js#L20-L23 · 24h expiry, auth.js#L12-L19 requireUser, used by the web pages, only lets a request through once optionalAuth has verified the token and set req.user: function requireUser(req, res, next) { if (!req.user) { return res.redirect('/login'); } next(); } src/routes/pages.js#L10-L15 Invitation-only registration ✅. Can someone register without an invitation, reuse one, or use someone else's? No. Codes are 128-bit random and only their bcrypt hash is stored, so even a database leak doesn't reveal a usable code. Each code is tied to one email and expires after 7 days, and creating a new one voids the old ones: await Invitation.updateMany( { email: normalized, used: false }, { $set: { used: true } } ); const plaintext = crypto.randomBytes(16).toString('hex'); const codeHash = await bcrypt.hash(plaintext, 10); const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); src/store/directoryStore.js#L226-L232 At registration, only an unused, unexpired code for that email is accepted, and the match is marked used so it works once: const invitations = await Invitation.find({ email: normalized, used: false, expiresAt: { $gt: new Date() } }).sort({ expiresAt: -1 }); ... inv.used = true; await inv.save(); src/store/directoryStore.js#L254-L264 And the new account's role is hard-coded, so you can't register yourself as an admin. A second registration with the same email is blocked by the pre-checks and by the unique index on email: const user = await db.createUser(email, passwordHash, 'employee', null); src/routes/auth.js#L76 · duplicate checks, auth.js#L50-L64 · unique index, directoryStore.js#L6 Vertical privilege escalation (employee → admin) ✅. The admin API router applies both middlewares to every route, and requireAdmin checks the role that requireAuth just loaded from the database: router.use(requireAuth, requireAdmin); src/routes/admin.js#L11 const requireAdmin = async (req, res, next) => { if (!req.user || req.user.role !== 'admin') { return res.status(403).json({ error: 'Admin access required' }); } next(); }; src/middleware/auth.js#L57-L62 The /admin* pages don't use that router, so each handler checks the role itself (pages.js#L85, #L106, #L128, #L140): router.post('/admin/promote/:id', requireUser, async (req, res, next) => { try { if (req.user.role !== 'admin') { return res.status(403).send('Forbidden'); } await db.updateUserRole(req.params.id, 'admin'); res.redirect('/admin'); src/routes/pages.js#L126-L132 Keep that last detail in mind: the check is "is this request from an admin?", and it is. Horizontal access to salaries ✅. getDirectoryForViewer (src/store/directoryStore.js#L210-L222, shown in story 5) returns maskEmployee for other people's rows. It builds a new object from the public fields only, so salary and email can't leak by accident: maskEmployee(emp) { return { id: emp.id, name: emp.name, title: emp.title, manager: emp.manager, deskNumber: emp.deskNumber, location: emp.location, phone: emp.phone }; } src/store/directoryStore.js#L198-L208 The self-edit routes look up the record from req.user.id (src/routes/pages.js#L64, src/routes/employees.js#L30), never from an ID in the request, and the API rejects salary outright: if (Object.prototype.hasOwnProperty.call(req.body, 'salary')) { return res.status(403).json({ error: 'Cannot update salary through this endpoint' }); } src/routes/employees.js#L35-L37 CSRF ✅. csurf checks a _csrf token on every state-changing request that isn't exempt. Requests with an Authorization: Bearer header skip it, which is safe: a page on another site can't make the browser add that header. Removing the _csrf field from an invite request in Burp Repeater gets Invalid or missing CSRF token. The cookie is also SameSite=Lax. function csrfMiddleware(req, res, next) { const auth = req.headers.authorization; const hasBearer = auth && String(auth).startsWith('Bearer '); if (hasBearer && !['GET', 'HEAD', 'OPTIONS'].includes(req.method)) { return next(); } if (req.method === 'POST' && CSRF_EXEMPT_AUTH_POST.has(req.path)) { return next(); } return csrfProtection(req, res, next); } app.js#L36-L46 One caveat for later: a CSRF token only stops requests from another origin. A script running on this origin can read the token like any form on the page does. Mass assignment ✅. Can an employee write fields they shouldn't, like salary, email or userId? No. There are two allowlists. The employee routes copy only the six public fields (src/routes/pages.js#L68-L74, src/routes/employees.js#L39-L45, shown in story 4). The store then copies only its own fixed list of keys, so unknown fields like role or _id never reach MongoDB: const allowed = [ 'name', 'title', 'manager', 'deskNumber', 'location', 'phone', 'salary', 'userId', 'email' ]; src/store/directoryStore.js#L160-L170 salary, userId and email are on the store's list, but only the admin API passes the raw body to it, and that route sits behind requireAdmin: router.patch('/employees/:id', async (req, res) => { try { const updated = await db.updateEmployeeById(req.params.id, req.body); src/routes/admin.js#L47-L49 This matches the OWASP Mass Assignment cheat sheet's advice to allowlist the bindable fields. 💉 Source-to-sink S2a, NoSQL injection in filters ✅. The classic attack sends an object instead of a string, for example {"email": {"$ne": null}}. express-mongo-sanitize strips $-prefixed and dotted keys from the body, query and params before any handler runs: app.use(mongoSanitize()); app.js#L66 On top of that, findUserByEmail calls email.toLowerCase(), which throws on an object, and findUserById goes through Mongoose, which casts the ID to an ObjectId and rejects anything else: async findUserByEmail(email) { const doc = await User.findOne({ email: email.toLowerCase().trim() }); return toUserDto(doc); } async findUserById(id) { const doc = await User.findById(id); return toUserDto(doc); } src/store/directoryStore.js#L86-L94 S2b, NoSQL injection in updates ✅. updateEmployeeById builds a brand-new update object from a fixed list of keys, so a top-level $unset or $rename is never copied in, and mongo-sanitize strips nested $ keys too: const update = {}; for (const key of allowed) { if (Object.prototype.hasOwnProperty.call(patch, key)) { ... update[key] = patch[key]; } } const doc = await Employee.findByIdAndUpdate(id, update, { new: true, runValidators: true }); src/store/directoryStore.js#L171-L186 S3, open redirect ✅. Every res.redirect target is a hard-coded relative path like /login or /profile?saved=1. User data only goes into the query string, and only after encodeURIComponent, so it can't change where the browser goes: return res.redirect('/register?error=' + encodeURIComponent('All fields are required')); ... res.redirect('/profile?saved=1'); src/routes/auth.js#L45 · src/routes/pages.js#L76 S1, XSS: this is where it breaks ❌. Go back to the template config from story 1: nunjucks.configure(viewsPath, { autoescape: false, app.js#L56-L57 Autoescaping is the control that turns < into <, " into " and so on whenever a value is printed with {{ }}. It's exactly what the OWASP XSS Prevention cheat sheet calls output encoding. Nunjucks turns it on by default, so this line switches it off for every template at once. Combine that with story 4 (Name is stored exactly as typed) and story 5 (Name is printed into the directory table and the admin page), and a profile field becomes HTML in the admin's browser. > 💡 Reviewer's lesson: when you see a template engine, a UI framework or a markdown renderer, check its escaping setting before you read a single template. One global flag can undo every {{ value }} in the app, and it's a one-line change that's easy to miss in review. --- 6. 🧪 Exploitation Why a plain <script> works Where the value lands decides what the payload has to look like: - The Directory, <td>{{ emp.name }}</td>: the value is element text, so a plain <script>…</script> becomes a real script tag. No breakout is needed. - The admin page, <input value="{{ emp.name }}">: the value is inside a quoted attribute, so a payload needs a leading "> to close the attribute and the tag first. - Your own profile page: the same attribute context, which is why the payload doesn't fire while you're typing it. PoC 1. The employee puts a script in their Name. On My profile, they set Name to test2<script>alert('XSS')</script> and click Save. The server stores it as typed. !The employee stores a script tag in the Name field of their profile 2. The admin logs in and opens the Directory. Admins land on /admin after login (top right: admin@localhost). From there they click Directory, an ordinary part of their day. !The admin is logged in on /admin and clicks Directory 3. The script runs in the admin's browser. The alert comes from localhost:3000, the app's own origin, so the attacker's code is running inside the admin's session. !An alert box saying XSS pops up on localhost:3000 for the admin What a real payload can do An alert is only the proof. A real payload can do anything the admin's page can do: - Read the admin's CSRF token from GET /api/csrf-token, or from the hidden _csrf fields on /admin. - Send admin requests as the admin. The session cookie is httpOnly, so the script can't read it, but it doesn't need to: the browser attaches it to every request the script sends. - Promote the attacker. With the token and the cookie, a POST /admin/promote/<attacker-id> passes requireUser, the role check and the CSRF check, because it really does come from the admin's browser. - Upgrade the attacker's existing session. requireAuth reloads the role from the database, so the attacker's own token becomes an admin token immediately. Impact: any employee becomes an admin the next time an admin opens the Directory. From there they can read and change every salary, delete records, create invitations and promote others. The same payload also runs for every employee who opens the directory. Classification: Stored Cross-Site Scripting, CWE-79, caused by disabled output encoding, CWE-116. It turns into privilege escalation because the victim is an admin. --- 7. 🛠️ The fix Primary fix: turn output encoding back on. Delete the line or set it to true: nunjucks.configure(viewsPath, { - autoescape: false, + autoescape: true, express: app, noCache: process.env.NODE_ENV !== 'production' }); Here is the real admin.html rendering the same kind of payload, before and after: autoescape:false -> <input type="text" name="location" value=""><script>alert(1)</script>"> autoescape:true -> <input type="text" name="location" value=""><script>alert(1)</script>"> Nunjucks' escaping encodes & " ' < >, which covers both places this app prints values: element text and double-quoted attributes. None of the templates intentionally output HTML, so nothing needs | safe. Defense-in-depth (do these too): 1. Add a Content Security Policy. None of the templates use inline <script>, so a policy like script-src 'self'; object-src 'none'; base-uri 'none' (for example with helmet) blocks this payload without breaking the UI. 2. Validate profile fields when they come in. Enforce lengths and per-field formats (phone, desk number), and reject < and > in plain-text fields. This is a safety net, not the fix: output encoding is still required. If you ever need rich text, use an allowlist sanitizer like DOMPurify instead of | safe. 3. Make privilege changes harder to ride. Require re-authentication or a confirmation step for Make admin, and audit-log role changes. 4. Guard the config in CI. No public Semgrep ruleset flags autoescape: false for Nunjucks, but a custom rule takes a few lines: rules: - id: nunjucks-autoescape-disabled languages: [javascript, typescript] severity: ERROR message: Nunjucks is configured with autoescape disabled, so every {{ value }} is written into the HTML unencoded. pattern-either: - pattern: "$NUNJUCKS.configure(..., {..., autoescape: false, ...})" - pattern: "new $NUNJUCKS.Environment(..., {..., autoescape: false, ...})" nunjucks-autoescape-disabled.yaml It flags the root cause directly: $ semgrep scan --metrics=off \ --config solutions/008-employee-directory/nunjucks-autoescape-disabled.yaml \ challenges/008-employee-directory challenges/008-employee-directory/app.js ❯❯❱ solutions.008-employee-directory.nunjucks-autoescape-disabled 56┆ nunjucks.configure(viewsPath, { 57┆ autoescape: false, 58┆ express: app, 59┆ noCache: process.env.NODE_ENV !== 'production' 60┆ }); The rule points at the config, not at each {{ emp.name }}, which is the right place: one flag turns off encoding for every template. A taint rule wouldn't find this anyway, because the payload is saved to MongoDB in one request and rendered in another. --- 🌍 Real-world examples - Stored XSS to Privilege Escalation to Admin Takeover to Data Breach — Ahmad Halabi (Aug 2025). A bug bounty write-up on a HackerOne financial program. An employee could only see the loan they were invited to, but the loan's purchase description was rendered without encoding. The researcher stored a <script src=…> payload there and invited the manager to review the loan. When the manager opened it, the script read the manager's CSRF token and replayed the role-change request, making the employee a manager. Almost exactly this challenge's chain. - From a Sanitized Name Field to One-Click Account Takeover — zere.es (2026). The server stripped HTML from a name field, but the front-end decoded HTML entities and wrote the value with innerHTML, so entity-encoded markup survived and ran. The author chained it with login CSRF and OAuth-code theft for one-click takeover. A different sink with the same lesson: filtering input doesn't help when the output isn't encoded for its context. 📚 More resources - CWE-79: Cross-site Scripting and CWE-116: Improper Encoding or Escaping of Output - OWASP Cross Site Scripting Prevention Cheat Sheet - Nunjucks: Autoescaping and configure options - PortSwigger Web Security Academy: Stored XSS - PortSwigger lab: Exploiting XSS to bypass CSRF defenses - MDN: Content Security Policy --- 🎯 Takeaway Employee Directory's server-side authorization is sound: role checks, salary masking, allowlists and CSRF tokens all hold, and httpOnly keeps the session cookie out of reach. One template setting makes them irrelevant. They all still run, but once attacker script runs on your origin in an admin's browser, every check sees a legitimate admin request. Whenever something renders in the browser, whether it's React, Angular, a template engine or a markdown renderer, check that user input goes through HTML escaping on the way out. Keep autoescape on, and treat any field a low-privileged user writes and a high-privileged user reads as a trust boundary. --- 🔔 What's next Challenge #9: ProjectHub is live now. Can you find the bug before the solution drops? 👉 github.com/mohamed-osama-aboelkheir/the-secure-co… New challenges land every two weeks, and each drop includes the previous challenge's solution. Watch the repo (Watch → Custom → Releases) to get notified, and subscribe to AppSec Untangled on YouTube for the video walkthroughs. If you found the bug yourself, or took a different route to it, tell me in the comments 👇
Advisories oficiales de DOMPurify: github.com/cure53/DOMPurify/security/advisories/G… github.com/cure53/DOMPurify/security/advisories/G… Release 3.4.17: github.com/cure53/DOMPurify/releases/tag/3.4.17
I've found XSS in DOMPurify: github.com/cure53/DOMPurify/security/advisories/G…