Derive uploaded file extension from mimetype, not originalname (fixes #11) #18

wtclaude · 2026-07-03T02:42:31Z

wtclaude commented

2026-07-03 02:42:31 +00:00

Summary

Fixes #11. The multer filename callback stored the file under path.extname(file.originalname), while the fileFilter only checked file.mimetype — the client-supplied multipart Content-Type, which is spoofable.

An authenticated uploader could therefore send Content-Type: image/png (passes the filter) with originalname: x.html. The file was saved as <ts>-<rand>.html and served from /uploads/ by express.static, which sets Content-Type: text/html from the extension. With CSP disabled and crossOriginResourcePolicy: cross-origin, that is same-origin stored HTML/JS execution — cookie theft / admin session hijack.

What changed

`server/src/router/v1/admin/admin.routes.js`

Added a MIME_EXT whitelist mapping each accepted image mimetype to the extension the file is stored under.
filename now derives the extension from MIME_EXT[file.mimetype], never from originalname. The random component uses crypto.randomBytes(8) instead of Math.random().
fileFilter now keys off the same MIME_EXT map, so the filter and the stored extension share a single source of truth — only a mimetype we can map to a safe extension is accepted.

const MIME_EXT = {
  'image/png': '.png', 'image/jpeg': '.jpg', 'image/gif': '.gif',
  'image/webp': '.webp', 'image/avif': '.avif',
}
// filename:
const ext = MIME_EXT[file.mimetype] || ''
cb(null, `${Date.now()}-${crypto.randomBytes(8).toString('hex')}${ext}`)
// fileFilter:
if (MIME_EXT[file.mimetype]) cb(null, true)
else cb(new Error('Only image uploads are allowed'))

`server/src/app.js`

/uploads is now served with an explicit X-Content-Type-Options: nosniff via express.static(..., { setHeaders }).

Why this approach

Extension from a validated whitelist, not originalname — this is the core fix. Even though the mimetype itself is spoofable, the worst an attacker can achieve is picking which image extension in the whitelist their file is stored as. There is no path to an .html/.svg/.js extension, so express.static can never serve the file as an executable type. This neutralizes the vulnerability at the point where the dangerous value (the extension) is chosen.
Single MIME_EXT source of truth — folding the filter and the extension mapping into one object removes the mismatch that made the original code fragile (filter regex vs. originalname extension) and guarantees every stored file has a known-safe extension. An unmapped mimetype is rejected outright rather than being stored with an empty extension.
crypto.randomBytes over Math.random() — Math.random() is not cryptographically random; predictable upload names are a (minor) information-leak / overwrite risk. randomBytes(8) is the standard fix and matches the issue's suggestion.
Explicit nosniff on /uploads (defense in depth) — helmet already sets X-Content-Type-Options: nosniff globally, so this is redundant today, but setting it directly on the untrusted-file handler keeps the protection in place even if the global helmet config is later changed, and documents intent at the point it matters. I did not add Content-Disposition: attachment, since uploaded images are embedded inline in posts/wiki via <img> and forcing downloads would be a behavior regression; the extension fix already closes the execution vector.

Testing

node -c syntax check on both changed files.
Traced the attack: image/png + originalname x.html now stores <ts>-<hex>.png; a non-image mimetype is rejected with 400; legitimate png/jpeg/gif/webp/avif uploads store with their correct extension.

Notes

Backend only, no schema or client changes. Existing upload API responses ({ image_url } / { url }) are unchanged. Branched from current main.

## Summary Fixes #11. The multer `filename` callback stored the file under `path.extname(file.originalname)`, while the `fileFilter` only checked `file.mimetype` — the client-supplied multipart `Content-Type`, which is spoofable. An authenticated uploader could therefore send `Content-Type: image/png` (passes the filter) with `originalname: x.html`. The file was saved as `<ts>-<rand>.html` and served from `/uploads/` by `express.static`, which sets `Content-Type: text/html` from the extension. With CSP disabled and `crossOriginResourcePolicy: cross-origin`, that is **same-origin stored HTML/JS execution** — cookie theft / admin session hijack. ## What changed ### `server/src/router/v1/admin/admin.routes.js` - Added a `MIME_EXT` whitelist mapping each accepted image mimetype to the extension the file is stored under. - **`filename`** now derives the extension from `MIME_EXT[file.mimetype]`, never from `originalname`. The random component uses `crypto.randomBytes(8)` instead of `Math.random()`. - **`fileFilter`** now keys off the same `MIME_EXT` map, so the filter and the stored extension share a single source of truth — only a mimetype we can map to a safe extension is accepted. ```js const MIME_EXT = { 'image/png': '.png', 'image/jpeg': '.jpg', 'image/gif': '.gif', 'image/webp': '.webp', 'image/avif': '.avif', } // filename: const ext = MIME_EXT[file.mimetype] || '' cb(null, `${Date.now()}-${crypto.randomBytes(8).toString('hex')}${ext}`) // fileFilter: if (MIME_EXT[file.mimetype]) cb(null, true) else cb(new Error('Only image uploads are allowed')) ``` ### `server/src/app.js` - `/uploads` is now served with an explicit `X-Content-Type-Options: nosniff` via `express.static(..., { setHeaders })`. ## Why this approach - **Extension from a validated whitelist, not `originalname`** — this is the core fix. Even though the mimetype itself is spoofable, the *worst* an attacker can achieve is picking which image extension in the whitelist their file is stored as. There is no path to an `.html`/`.svg`/`.js` extension, so `express.static` can never serve the file as an executable type. This neutralizes the vulnerability at the point where the dangerous value (the extension) is chosen. - **Single `MIME_EXT` source of truth** — folding the filter and the extension mapping into one object removes the mismatch that made the original code fragile (filter regex vs. `originalname` extension) and guarantees every stored file has a known-safe extension. An unmapped mimetype is rejected outright rather than being stored with an empty extension. - **`crypto.randomBytes` over `Math.random()`** — `Math.random()` is not cryptographically random; predictable upload names are a (minor) information-leak / overwrite risk. `randomBytes(8)` is the standard fix and matches the issue's suggestion. - **Explicit `nosniff` on `/uploads` (defense in depth)** — helmet already sets `X-Content-Type-Options: nosniff` globally, so this is redundant *today*, but setting it directly on the untrusted-file handler keeps the protection in place even if the global helmet config is later changed, and documents intent at the point it matters. I did **not** add `Content-Disposition: attachment`, since uploaded images are embedded inline in posts/wiki via `<img>` and forcing downloads would be a behavior regression; the extension fix already closes the execution vector. ## Testing - `node -c` syntax check on both changed files. - Traced the attack: `image/png` + `originalname x.html` now stores `<ts>-<hex>.png`; a non-image mimetype is rejected with 400; legitimate png/jpeg/gif/webp/avif uploads store with their correct extension. ## Notes Backend only, no schema or client changes. Existing upload API responses (`{ image_url }` / `{ url }`) are unchanged. Branched from current `main`.

wtclaude added 1 commit 2026-07-03 02:42:32 +00:00

Derive uploaded file extension from mimetype, not originalname (#11 ) e84835a0fb

The multer filename kept path.extname(file.originalname), while the
fileFilter only checked the spoofable client-supplied mimetype. An
attacker could send Content-Type: image/png with originalname x.html,
landing an .html file in /uploads that express.static serves as
text/html — same-origin stored XSS.

- Store the extension from a whitelist keyed by the accepted mimetype
  (MIME_EXT), never from originalname. The fileFilter uses the same map
  as its single source of truth, so only mimetypes with a safe mapped
  extension pass.
- Use crypto.randomBytes for the random filename component.
- Serve /uploads with an explicit X-Content-Type-Options: nosniff
  (defense in depth alongside helmet's global setting).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

whitlocktech requested review from whitlocktech 2026-07-03 02:43:58 +00:00

whitlocktech approved these changes 2026-07-03 02:44:23 +00:00

whitlocktech merged commit ad9c556c9a into main

2026-07-03 02:44:30 +00:00

whitlocktech referenced this issue from a commit

2026-07-03 02:44:30 +00:00

Merge pull request 'Derive uploaded file extension from mimetype, not originalname (fixes #11)' (#18) from fix/upload-extension-xss into main

wtclaude referenced this pull request

2026-07-29 02:07:07 +00:00

feat(shard): ingest points.board and publish the leaderboards #114

Sign in to join this conversation.