Skip to content

Commit 1310c9f

Browse files
fix(uploads): allowlist Content-Type on serve paths — defense-in-depth against stored-XSS (#3729) (#3732)
* fix(uploads): allowlist Content-Type on serve paths to prevent stored-XSS (#3729) Defense-in-depth backstop: the download handler now validates the stored content-type against SAFE_MIME (get) / SAFE_IMAGE_MIME (getSharp) before echoing it, falling back to application/octet-stream / image/jpeg respectively. Adds Content-Disposition: attachment on the private download path. Covers both req.upload.contentType and req.upload.metadata.contentType. 15 new unit tests assert safe MIMEs pass through and dangerous ones are downgraded. * fix(uploads): normalize MIME before allowlist check + harden pipe mocks + expand getSharp coverage - Add normalizeMime() helper: lowercase + strip ; params - Apply normalizeMime() in both get and getSharp before allowlist lookup - Add @returns JSDoc to getSharp per repo guideline - Fix stream.pipe mock in makeStream() to return dest (chained pipe support) - Fix sharp().pipe mock to return dest (stream.pipe(transform).pipe(res) chain) - Remove per-test stream.pipe overrides (centralized in makeStream) - Add getSharp tests: metadata.contentType safe/dangerous, missing fallback, uppercase MIME, MIME-with-params normalization - Add get tests: uppercase MIME, MIME-with-params normalization Addresses all Copilot review threads on PR #3732.
1 parent b08e9a3 commit 1310c9f

2 files changed

Lines changed: 392 additions & 2 deletions

File tree

modules/uploads/controllers/uploads.controller.js

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,27 @@ import errors from '../../../lib/helpers/errors.js';
99
import responses from '../../../lib/helpers/responses.js';
1010
import UploadsService from '../services/uploads.service.js';
1111

12+
/**
13+
* Allowlisted MIME types for private download (get).
14+
* Defense-in-depth: prevents stored-XSS when a downstream kind permits a dangerous MIME.
15+
*/
16+
const SAFE_MIME = new Set(['image/jpeg', 'image/jpg', 'image/png', 'image/gif', 'image/webp', 'application/pdf']);
17+
18+
/**
19+
* Allowlisted MIME types for public image serving (getSharp).
20+
* Restricted to image types — the sharp pipeline only handles images.
21+
*/
22+
const SAFE_IMAGE_MIME = new Set(['image/jpeg', 'image/jpg', 'image/png', 'image/gif', 'image/webp']);
23+
24+
/**
25+
* Normalize a raw Content-Type value for safe allowlist comparison.
26+
* MIME types are case-insensitive and may include parameters (e.g. `image/jpeg; charset=binary`).
27+
* Strip any `;` parameter segment, lowercase, and trim before checking the allowlist.
28+
* @param {string} raw - Raw content-type string.
29+
* @returns {string} Normalized MIME type (e.g. `image/jpeg`).
30+
*/
31+
const normalizeMime = (raw) => String(raw).toLowerCase().split(';')[0].trim();
32+
1233
/**
1334
* @desc Endpoint to get an upload by fileName
1435
* @param {Object} req - Express request object
@@ -21,8 +42,11 @@ const get = async (req, res) => {
2142
stream.on('error', (err) => {
2243
responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
2344
});
24-
const contentType = req.upload.contentType || req.upload.metadata?.contentType || 'application/octet-stream';
45+
const raw = req.upload.contentType || req.upload.metadata?.contentType || 'application/octet-stream';
46+
const norm = normalizeMime(raw);
47+
const contentType = SAFE_MIME.has(norm) ? norm : 'application/octet-stream';
2548
res.set('Content-Type', contentType);
49+
res.set('Content-Disposition', 'attachment');
2650
if (req.upload.length) res.set('Content-Length', req.upload.length);
2751
stream.pipe(res);
2852
} catch (err) {
@@ -34,6 +58,7 @@ const get = async (req, res) => {
3458
* @desc Endpoint to get an upload by fileName with sharp options
3559
* @param {Object} req - Express request object
3660
* @param {Object} res - Express response object
61+
* @returns {Promise<void>} Resolves when the image stream has been piped to the response.
3762
*/
3863
const getSharp = async (req, res) => {
3964
try {
@@ -42,7 +67,9 @@ const getSharp = async (req, res) => {
4267
stream.on('error', (err) => {
4368
responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
4469
});
45-
const contentType = req.upload.contentType || req.upload.metadata?.contentType || 'application/octet-stream';
70+
const raw = req.upload.contentType || req.upload.metadata?.contentType || 'application/octet-stream';
71+
const norm = normalizeMime(raw);
72+
const contentType = SAFE_IMAGE_MIME.has(norm) ? norm : 'image/jpeg';
4673
res.set('Content-Type', contentType);
4774
switch (req.sharpOption) {
4875
case 'blur':

0 commit comments

Comments
 (0)