diff --git a/backend/__tests__/integration/businessProfileLogoUpload.test.js b/backend/__tests__/integration/businessProfileLogoUpload.test.js
new file mode 100644
index 00000000..bf1c93e3
--- /dev/null
+++ b/backend/__tests__/integration/businessProfileLogoUpload.test.js
@@ -0,0 +1,195 @@
+/**
+ * POST /api/admin/business-profile/logo and PUT /api/admin/business-profile
+ * — GHSA-6wrv-9pr4-hhmw regression coverage.
+ *
+ * The upload route used to take the stored file extension straight from
+ * the client-supplied filename and only checked `file.mimetype` against an
+ * allowlist — a file could declare an image MIME type while carrying a
+ * `.html`/`.js` extension and arbitrary content, land in the same-origin
+ * `/uploads/logos` static mount, and execute as script. The mass-assignable
+ * `logoPath` field on PUT compounded it: an attacker could point the
+ * "logo" at any other uploaded file.
+ *
+ * These tests pin:
+ * (a) a MIME/extension mismatch is rejected at upload,
+ * (b) the extension actually written to disk always matches the
+ * validated MIME type, never the client-supplied filename,
+ * (c) legitimate PNG/JPEG/SVG uploads still succeed,
+ * (d) `logoPath` on PUT cannot be set to an arbitrary string pointing at
+ * another file, only to a path the upload route itself produced.
+ *
+ * Defense-in-depth (not a re-opening of the above): fileFilter only pairs
+ * the claimed MIME type against the extension — it can't see the bytes,
+ * since it runs before multer finishes writing the stream to disk. A file
+ * whose declared MIME/extension pair is valid but whose actual content
+ * doesn't match (e.g. a PNG-declared upload that isn't really a PNG) is
+ * now caught by validateFileContent() (magic-number check) after multer
+ * writes it, closing the gap where declared-vs-actual content diverges.
+ */
+
+const path = require('path');
+const fs = require('fs');
+const os = require('os');
+
+const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-bplogo-test-'));
+process.env.NODE_ENV = 'test';
+process.env.TEST_DATABASE_PATH = path.join(tmpDir, 'db.sqlite');
+process.env.STORAGE_PATH = path.join(tmpDir, 'storage');
+fs.mkdirSync(process.env.STORAGE_PATH, { recursive: true });
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'bplogo-route-test-secret';
+
+const request = require('supertest');
+const {
+ bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken, buildRouteApp,
+} = require('./helpers/crmDb');
+
+// Real magic-number-prefixed payloads, for content-sniffing to accept.
+const REAL_PNG_BYTES = Buffer.concat([
+ Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]),
+ Buffer.from('not a real png body, but the header is real'),
+]);
+const REAL_JPEG_BYTES = Buffer.concat([
+ Buffer.from([0xFF, 0xD8, 0xFF]),
+ Buffer.from('not a real jpeg body, but the header is real'),
+]);
+
+describe('business profile — logo upload content/extension validation', () => {
+ let db;
+ let cleanup;
+ let app;
+ let token;
+
+ const uploadLogo = (buffer, filename, mimetype) => request(app)
+ .post('/api/admin/business-profile/logo')
+ .set('Authorization', `Bearer ${token}`)
+ .attach('logo', buffer, { filename, contentType: mimetype });
+
+ const put = (payload) => request(app)
+ .put('/api/admin/business-profile')
+ .set('Authorization', `Bearer ${token}`)
+ .send(payload);
+
+ const get = () => request(app)
+ .get('/api/admin/business-profile')
+ .set('Authorization', `Bearer ${token}`);
+
+ const profileOf = (res) => (res.body.data || res.body).profile;
+
+ beforeAll(async () => {
+ ({ db, cleanup } = await bootCrmDb());
+ const { adminId } = await seedMinimal(db);
+ await assignAdminRole(db, adminId, 'super_admin');
+ token = mintAdminToken(adminId);
+ app = buildRouteApp('/api/admin/business-profile', require('../../src/routes/adminBusinessProfile'));
+ }, 120000);
+
+ afterAll(async () => {
+ if (cleanup) await cleanup();
+ });
+
+ // fileFilter rejections surface via Express's generic error handler
+ // (the pre-existing behaviour of every sibling logo/favicon upload
+ // route in this codebase — none of them special-case multer's
+ // fileFilter `Error` into a 400 either), so the status code itself
+ // can be 400 or 500 depending on environment. What actually matters
+ // for GHSA-6wrv-9pr4-hhmw is that the request never succeeds and
+ // nothing with the dangerous extension is ever written to disk.
+ const logosDirFiles = () => {
+ const logosDir = path.join(process.env.STORAGE_PATH, 'uploads', 'logos');
+ return fs.existsSync(logosDir) ? fs.readdirSync(logosDir) : [];
+ };
+
+ it('rejects an HTML/script payload disguised as an image via mismatched extension', async () => {
+ const evil = Buffer.from('');
+ const res = await uploadLogo(evil, 'evil.html', 'image/svg+xml');
+ expect(res.status).not.toBe(200);
+ expect(logosDirFiles().some((f) => f.endsWith('.html'))).toBe(false);
+ });
+
+ it('rejects a .js file disguised with an image MIME type', async () => {
+ const evil = Buffer.from('alert(1)');
+ const res = await uploadLogo(evil, 'evil.js', 'image/png');
+ expect(res.status).not.toBe(200);
+ expect(logosDirFiles().some((f) => f.endsWith('.js'))).toBe(false);
+ });
+
+ it('rejects a disallowed MIME type outright', async () => {
+ const res = await uploadLogo(Buffer.from('whatever'), 'file.pdf', 'application/pdf');
+ expect(res.status).not.toBe(200);
+ expect(logosDirFiles().some((f) => f.endsWith('.pdf'))).toBe(false);
+ });
+
+ it('accepts a legitimate PNG upload and stores it with a .png extension', async () => {
+ const res = await uploadLogo(REAL_PNG_BYTES, 'logo.png', 'image/png');
+ expect(res.status).toBe(200);
+ const logoPath = (res.body.data || res.body).logoPath;
+ expect(logoPath).toMatch(/^\/uploads\/logos\/pdf-logo-\d+\.png$/);
+
+ const onDisk = path.join(process.env.STORAGE_PATH, logoPath.replace(/^\//, ''));
+ expect(fs.existsSync(onDisk)).toBe(true);
+
+ expect(profileOf(await get()).logoPath).toBe(logoPath);
+ });
+
+ it('accepts a legitimate JPEG upload and stores it with a .jpg extension', async () => {
+ const res = await uploadLogo(REAL_JPEG_BYTES, 'logo.jpg', 'image/jpeg');
+ expect(res.status).toBe(200);
+ const logoPath = (res.body.data || res.body).logoPath;
+ expect(logoPath).toMatch(/^\/uploads\/logos\/pdf-logo-\d+\.jpg$/);
+ });
+
+ it('rejects a PNG-declared upload whose bytes are not actually a PNG, and leaves nothing on disk', async () => {
+ const before = logosDirFiles();
+ const res = await uploadLogo(Buffer.from('totally not a png'), 'logo.png', 'image/png');
+ expect(res.status).toBe(400);
+ expect(res.body.error).toMatch(/content does not match/i);
+
+ // No new file left behind: the rejected upload's own file was cleaned
+ // up, and every other file on disk (if any) is unchanged.
+ expect(logosDirFiles()).toEqual(before);
+ });
+
+ it('accepts a legitimate SVG upload and always stores it with a .svg extension, even under a spoofed filename', async () => {
+ const svg = Buffer.from('');
+ // Client-declared filename ext is .svg here to pass validateFileType
+ // (mismatched ext is covered by the rejection tests above); the point
+ // of this test is that the ON-DISK extension comes from the MIME type
+ // lookup table, not path.extname(originalname).
+ const res = await uploadLogo(svg, 'vector-logo.svg', 'image/svg+xml');
+ expect(res.status).toBe(200);
+ const logoPath = (res.body.data || res.body).logoPath;
+ expect(logoPath).toMatch(/^\/uploads\/logos\/pdf-logo-\d+\.svg$/);
+ });
+
+ it('rejects logoPath on PUT set to an arbitrary string pointing at another file', async () => {
+ const before = profileOf(await get()).logoPath;
+
+ const res = await put({ logoPath: '/uploads/logos/cms-somepage-1234.png' });
+ expect(res.status).toBe(400);
+
+ expect(profileOf(await get()).logoPath).toBe(before);
+ });
+
+ it('rejects logoPath on PUT with a path-traversal payload', async () => {
+ const res = await put({ logoPath: '/uploads/logos/../../../../etc/passwd' });
+ expect(res.status).toBe(400);
+ });
+
+ it('accepts logoPath on PUT when it matches the pattern this route itself writes', async () => {
+ const upload = await uploadLogo(REAL_PNG_BYTES, 'logo2.png', 'image/png');
+ const uploadedPath = (upload.body.data || upload.body).logoPath;
+
+ // Round-trip: PUT-ing back the exact value the upload endpoint
+ // returned (what the frontend's generic profile save does) must
+ // keep working.
+ const res = await put({ logoPath: uploadedPath });
+ expect(res.status).toBe(200);
+ expect(profileOf(await get()).logoPath).toBe(uploadedPath);
+ });
+
+ it('still allows clearing logoPath with an empty string', async () => {
+ const res = await put({ logoPath: '' });
+ expect(res.status).toBe(200);
+ expect(profileOf(await get()).logoPath).toBe('');
+ });
+});
diff --git a/backend/src/routes/adminBusinessProfile.js b/backend/src/routes/adminBusinessProfile.js
index 889f35d9..ef31a06e 100644
--- a/backend/src/routes/adminBusinessProfile.js
+++ b/backend/src/routes/adminBusinessProfile.js
@@ -23,6 +23,7 @@ const { requirePermission } = require('../middleware/permissions');
const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers');
const { getStoragePath } = require('../config/storage');
const { uploadedPdfLogoPath } = require('../utils/safePath');
+const { validateFileType, validateFileContent, ALLOWED_MEDIA_TYPES } = require('../utils/fileSecurityUtils');
const businessProfileService = require('../services/businessProfileService');
const { db } = require('../database/db');
const { validateIban } = require('../utils/iban');
@@ -95,6 +96,19 @@ const router = express.Router();
// but accepts SVG in addition to PNG / JPEG — the PDF renderer
// rasterises SVGs to PNG on the fly via resolveLogoFile() so the
// admin can drop a vector logo here and have it work in print.
+//
+// GHSA-6wrv-9pr4-hhmw: this route used to take the stored extension
+// straight from `file.originalname` and only checked `file.mimetype`
+// against an allowlist — a file could declare an image MIME type
+// while carrying a `.html`/`.js` extension and arbitrary content, get
+// served same-origin from /uploads/logos with that extension, and
+// execute as script in the browser. Fixed the same way every sibling
+// upload route (adminSettings.js, adminCMS.js) already does it:
+// `validateFileType()` pairs the claimed MIME type against the
+// extension, and the extension actually written to disk is looked up
+// from the validated MIME type — never taken from client input.
+const PDF_LOGO_ALLOWED_MIME_TYPES = ['image/png', 'image/jpeg', 'image/svg+xml'];
+
const pdfLogoStorage = multer.diskStorage({
destination: async (_req, _file, cb) => {
const dir = path.join(getStoragePath(), 'uploads/logos');
@@ -102,7 +116,11 @@ const pdfLogoStorage = multer.diskStorage({
cb(null, dir);
},
filename: (_req, file, cb) => {
- const ext = path.extname(file.originalname) || '.png';
+ // fileFilter (below) runs before this and already rejected any
+ // mimetype outside PDF_LOGO_ALLOWED_MIME_TYPES, so the lookup below
+ // always hits. The extension is derived from the validated MIME
+ // type, never from file.originalname.
+ const ext = ALLOWED_MEDIA_TYPES[file.mimetype]?.extensions[0] || '.png';
cb(null, `pdf-logo-${Date.now()}${ext}`);
},
});
@@ -113,9 +131,11 @@ const pdfLogoUpload = multer({
// array-indexed field names, so reject any bracket-index field name.
limits: { fileSize: 5 * 1024 * 1024, fieldArrayIndexLimit: 0 },
fileFilter: (_req, file, cb) => {
- const allowed = ['image/png', 'image/jpeg', 'image/svg+xml'];
- if (allowed.includes(file.mimetype)) cb(null, true);
- else cb(new Error('Only PNG, JPEG and SVG logos are allowed'));
+ if (validateFileType(file.originalname, file.mimetype, PDF_LOGO_ALLOWED_MIME_TYPES)) {
+ cb(null, true);
+ } else {
+ cb(new Error('Only PNG, JPEG and SVG logos are allowed'));
+ }
},
});
@@ -355,6 +375,19 @@ router.post(
return res.status(400).json({ error: 'No logo file uploaded' });
}
+ // fileFilter above only pairs the claimed MIME type against the
+ // extension — it runs on the in-flight stream, before any bytes are
+ // written, so it can't inspect content. Content-sniff the bytes multer
+ // just wrote to disk (magic numbers) before trusting them; SVG has no
+ // magic-number check (validateFileContent returns true for it), it's
+ // protected by the CSP header instead. Matches the cleanup-then-reject
+ // pattern createFileUploadValidator() uses for other upload routes.
+ const contentIsValid = await validateFileContent(req.file.path, req.file.mimetype);
+ if (!contentIsValid) {
+ try { await fs.unlink(req.file.path); } catch (_) { /* ignore */ }
+ return res.status(400).json({ error: 'File content does not match its declared type' });
+ }
+
// Clean up the previous PDF logo on disk if it was uploaded via
// this same endpoint (matches the pdf-logo-* prefix). We leave
// anything else untouched — the admin may have set logo_path to
@@ -428,7 +461,19 @@ router.put(
body('defaultLocale').optional({ values: 'falsy' }).isString().isLength({ max: 8 }),
body('defaultQrFormat').optional({ values: 'falsy' }).isIn(['swiss', 'epc', 'none']),
body('footerLine').optional({ values: 'falsy' }).isString().isLength({ max: 255 }),
- body('logoPath').optional({ values: 'falsy' }).isString().isLength({ max: 512 }),
+ // GHSA-6wrv-9pr4-hhmw: logoPath is mass-assignable here, so it must
+ // only ever be settable to a path the POST /logo upload route itself
+ // produced (or '' to clear it, allowed by `values: 'falsy'` above) —
+ // not an arbitrary string chaining in a file uploaded elsewhere.
+ // uploadedPdfLogoPath() is the same pattern check the delete/replace
+ // cleanup path already trusts to name a file this route wrote.
+ body('logoPath').optional({ values: 'falsy' }).isString().isLength({ max: 512 })
+ .custom((value) => {
+ if (!uploadedPdfLogoPath(value, getStoragePath())) {
+ throw new Error('logoPath must be a path produced by the logo upload endpoint');
+ }
+ return true;
+ }),
// Bundled-fonts dropdown (migration 121). Free-text upload field
// (pdfFontTtfPath, migration 103) was retired from the UI in
// favour of this dropdown; the column stays in the DB so any