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

Reviewed-on: UOM/website#18
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

server/src/app.js

@@ -51,8 +51,16 @@ const UPLOAD_DIR = process.env.UPLOAD_DIR || path.join(SERVER_ROOT, 'uploads')
 const CLIENT_DIST = path.join(REPO_ROOT, 'client', 'dist')
 fs.mkdirSync(UPLOAD_DIR, { recursive: true })
 
-// Uploaded images — always served, even during maintenance.
+// Uploaded images — always served, even during maintenance. Force nosniff so a
-app.use('/uploads', express.static(UPLOAD_DIR))
+// stored file is never interpreted as anything other than its declared type
+// (defense in depth alongside helmet's global X-Content-Type-Options, and in
+// case that global config is ever changed).
+app.use(
+  '/uploads',
+  express.static(UPLOAD_DIR, {
+    setHeaders: (res) => res.set('X-Content-Type-Options', 'nosniff'),
+  }),
+)
 
 // ── API ───────────────────────────────────────────────────────────────
 app.get('/api/health', (req, res) => res.json({ status: 'ok' }))

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

@@ -1,6 +1,7 @@
 const express = require('express')
 const path = require('path')
 const fs = require('fs')
+const crypto = require('crypto')
 const multer = require('multer')
 const { body, param } = require('express-validator')
 
@@ -23,18 +24,31 @@ const UPLOAD_DIR =
   process.env.UPLOAD_DIR || path.join(__dirname, '..', '..', '..', '..', 'uploads')
 fs.mkdirSync(UPLOAD_DIR, { recursive: true })
 
+// Whitelisted image mimetypes → the extension we store the file under. The
+// stored extension is derived from this map (keyed by the accepted mimetype),
+// never from originalname — so a spoofed `Content-Type: image/png` paired with
+// `originalname: x.html` can never land an executable .html file in /uploads.
+const MIME_EXT = {
+  'image/png': '.png',
+  'image/jpeg': '.jpg',
+  'image/gif': '.gif',
+  'image/webp': '.webp',
+  'image/avif': '.avif',
+}
+
 const storage = multer.diskStorage({
   destination: (req, file, cb) => cb(null, UPLOAD_DIR),
   filename: (req, file, cb) => {
-    const ext = path.extname(file.originalname).toLowerCase()
+    const ext = MIME_EXT[file.mimetype] || ''
-    cb(null, `${Date.now()}-${Math.round(Math.random() * 1e9)}${ext}`)
+    cb(null, `${Date.now()}-${crypto.randomBytes(8).toString('hex')}${ext}`)
   },
 })
 const upload = multer({
   storage,
   limits: { fileSize: 8 * 1024 * 1024 },
   fileFilter: (req, file, cb) => {
-    if (/^image\/(png|jpe?g|gif|webp|avif)$/.test(file.mimetype)) cb(null, true)
+    // Single source of truth: only mimetypes we can map to a safe extension pass.
+    if (MIME_EXT[file.mimetype]) cb(null, true)
     else cb(new Error('Only image uploads are allowed'))
   },
 })