fix(security): never serve a photo under its stored MIME, and stop trusting the chunked-upload type

chunked-upload/init stored the client-declared mimeType on the photo row and the gallery, secure-image and protected-image routes echoed it as Content-Type, so a JPEG/HTML polyglot declared as text/html rendered inline on the app origin for every guest. The admin photo route already resolved the type safely (#908 review); that logic now lives in utils/photoContentType and every serving route uses it. The chunked path derives the MIME from the filename extension and requires that extension to be on the admin allow-list, matching what the multipart path enforces through its multer fileFilter.
2026-09-03 10:48:44 +02:00
parent 3e46530072
commit 063977d97d
6 changed files with 139 additions and 78 deletions
@@ -0,0 +1,47 @@
+/**
+ * photos.mime_type is client-influenced (chunked uploads stored the declared
+ * type verbatim; the S3 importer stores whatever mime-types derives). Every
+ * serving route must go through resolvePhotoContentType so the header is
+ * always image/* or video/* and never the stored value as given.
+ */
+const fs = require('fs');
+const path = require('path');
+const { resolvePhotoContentType } = require('../../src/utils/photoContentType');
+
+describe('resolvePhotoContentType', () => {
+  it('never echoes a non-media stored MIME', () => {
+    expect(resolvePhotoContentType({ filename: 'a.jpg', mime_type: 'text/html' })).toBe('image/jpeg');
+    expect(resolvePhotoContentType({ filename: 'a', mime_type: 'text/html' })).toBe('image/jpeg');
+    expect(resolvePhotoContentType({ filename: 'a.gif', mime_type: 'application/javascript' })).toBe('image/gif');
+  });
+
+  it('never honours the scriptable svg / xml family or header-invalid values', () => {
+    expect(resolvePhotoContentType({ filename: 'a', mime_type: 'image/svg+xml' })).toBe('image/jpeg');
+    expect(resolvePhotoContentType({ filename: 'a', mime_type: 'image/x\r\nX-Injected: 1' })).toBe('image/jpeg');
+    expect(resolvePhotoContentType({ filename: 'a.mp4', mime_type: 'video/mp4\r\nX: y' })).toBe('video/mp4');
+  });
+
+  it('prefers the mapped extension for images and the stored type for videos', () => {
+    expect(resolvePhotoContentType({ filename: 'a.png', mime_type: 'image/jpeg' })).toBe('image/png');
+    expect(resolvePhotoContentType({ filename: 'a.mov', mime_type: null })).toBe('video/quicktime');
+    expect(resolvePhotoContentType({ filename: 'a.bin', media_type: 'video' })).toBe('video/mp4');
+    expect(resolvePhotoContentType({ filename: 'a', mime_type: 'image/avif' })).toBe('image/avif');
+    expect(resolvePhotoContentType({ filename: 'a.constructor', mime_type: null })).toBe('image/jpeg');
+  });
+});
+
+describe('serving routes use the resolver', () => {
+  const routes = ['gallery.js', 'secureImages.js', 'protectedImages.js', 'adminPhotos.js'];
+  it.each(routes)('%s sets no Content-Type from photo.mime_type directly', (name) => {
+    const src = fs.readFileSync(path.join(__dirname, '../../src/routes', name), 'utf8');
+    expect(src).not.toMatch(/'Content-Type':\s*photo\.mime_type/);
+    expect(src).not.toMatch(/set\('Content-Type',\s*photo\.mime_type\)/);
+    expect(src).toMatch(/resolvePhotoContentType\(photo\)/);
+  });
+
+  it('chunked-upload init derives the MIME from the allow-listed extension', () => {
+    const src = fs.readFileSync(path.join(__dirname, '../../src/routes/adminPhotos.js'), 'utf8');
+    expect(src).not.toMatch(/const \{ filename, fileSize, mimeType, totalChunks \} = req\.body/);
+    expect(src).toMatch(/allowedMimeTypes\.includes\(mimeType\)/);
+  });
+});
@@ -23,8 +23,10 @@ const {
  getMaxFileSizeBytes,
  getMaxVideoSizeBytes,
  DEFAULT_MAX_FILE_SIZE_MB,
-  DEFAULT_MAX_VIDEO_SIZE_MB
+  DEFAULT_MAX_VIDEO_SIZE_MB,
+  EXTENSION_TO_MIME
 } = require('../services/uploadSettings');
+const { resolvePhotoContentType } = require('../utils/photoContentType');
 const { processUploadedPhotos } = require('../services/photoProcessor');
 const chunkedUpload = require('../services/chunkedUploadService');
 const watermarkGeneratorService = require('../services/watermarkGeneratorService');
@@ -1165,7 +1167,7 @@ router.get('/:eventId/photos/:photoId/download', adminAuth, requirePermission('p
        return res.status(404).json({ error: 'Photo file not found' });
      }
      res.set({
-        'Content-Type': photo.mime_type || 'application/octet-stream',
+        'Content-Type': resolvePhotoContentType(photo),
        'Content-Length': stat.size,
        'Content-Disposition': contentDisposition,
      });
@@ -1182,7 +1184,7 @@ router.get('/:eventId/photos/:photoId/download', adminAuth, requirePermission('p
      return res.status(404).json({ error: 'Photo file not found' });
    }
    res.set({
-      'Content-Type': photo.mime_type || 'application/octet-stream',
+      'Content-Type': resolvePhotoContentType(photo),
      'Content-Disposition': contentDisposition,
    });
    res.sendFile(filePath);
@@ -1441,64 +1443,9 @@ router.get('/:eventId/photo/:photoId', adminAuth, requirePermission('photos.view
    const event = await db('events').where('id', eventId).first();
    const storageKey = resolvePhotoStorageKey(event, photo);

-    // Content-Type resolution (#908 + external review). Invariant: the
-    // header is ALWAYS image/* or video/*.
-    //  - photos.mime_type is never echoed verbatim unless it is a video/
-    //    type: the chunked-upload path stores the client-sent MIME
-    //    unvalidated, so a stored text/html served inline under the app
-    //    origin would be a same-origin XSS gift.
-    //  - Images ignore the stored value entirely — migration 039
-    //    backfilled image/jpeg onto every legacy row (PNGs included), so
-    //    the extension is the more trustworthy signal; normalized via the
-    //    shared map (image/jpg → image/jpeg), jpeg fallback when unknown.
-    //  - Videos prefer a stored video/ type, then the extension map
-    //    (.mov → video/quicktime, .webm → video/webm, …), then video/mp4.
-    //    The old ext-derived image/<ext> (image/mp4) is what made the
-    //    admin player's blob unplayable (#908).
-    const { EXTENSION_TO_MIME } = require('../services/uploadSettings');
-    const ext = path.extname(photo.filename).slice(1).toLowerCase();
-    // Own-property lookup (review): a client-controlled filename ending in
-    // .constructor / .__proto__ / .toString would otherwise return an
-    // inherited Object.prototype member, and the extMime.startsWith below
-    // would throw — a permanent 500 for that photo instead of the fallback.
-    const extMime = Object.prototype.hasOwnProperty.call(EXTENSION_TO_MIME, ext)
-      ? EXTENSION_TO_MIME[ext]
-      : null;
-    // Full-token validation, not just a prefix check: the stored value is
-    // client-controlled, and header-invalid characters (video/mp4\r\nX: y)
-    // would make setHeader throw — a permanent 500 for that photo. Bare
-    // 'video/' is equally invalid; both fall back to the extension map.
-    const storedVideoMime = photo.mime_type && /^video\/[\w.+-]+$/.test(photo.mime_type)
-      ? photo.mime_type
-      : null;
-    // Honor a stored image MIME for any header-safe RASTER type (#908
-    // review): the S3 auto-importer accepts arbitrary image/* from
-    // mime-types and stores it (avif/bmp/tiff/heic/apng/ico/jxl/…), and a
-    // hand-listed allowlist kept missing formats. Allow image/<token> but
-    // NEVER the scriptable svg / *+xml family (image/svg+xml executes
-    // inline). The strict token + anchors also block header injection
-    // (image/x\r\nY:). Migration 039's blanket image/jpeg backfill on
-    // legacy rows is why the mapped extension still wins ahead of this.
-    const storedImageMime =
-      photo.mime_type &&
-      /^image\/[\w.+-]+$/.test(photo.mime_type) &&
-      !/^image\/svg|xml/i.test(photo.mime_type)
-        ? photo.mime_type
-        : null;
-    const isVideo = photo.media_type === 'video' ||
-      Boolean(storedVideoMime) ||
-      Boolean(extMime && extMime.startsWith('video/'));
-    // Never interpolate the raw extension on the image side: it would
-    // synthesize image/svg+xml (scriptable inline) or header-invalid values
-    // from client-controlled chunked-upload filenames. Precedence is
-    // mapped-extension (also corrects the 039 legacy-jpeg backfill on PNGs)
-    // -> safe stored raster MIME (auto-imported avif/bmp/tiff) -> image/jpeg.
-    // A stored type outside the allowlist degrades to image/jpeg; browsers
-    // sniff image bytes in <img>/blob contexts, so a mislabel is harmless
-    // where an injected type is not.
-    const contentType = isVideo
-      ? storedVideoMime || (extMime && extMime.startsWith('video/') ? extMime : null) || 'video/mp4'
-      : (extMime && extMime.startsWith('image/') ? extMime : null) || storedImageMime || 'image/jpeg';
+    // Content-Type resolution (#908 + external review) lives in
+    // utils/photoContentType so the gallery routes apply the same rule.
+    const contentType = resolvePhotoContentType(photo);
    res.setHeader('Content-Type', contentType);
    res.setHeader('Cache-Control', 'private, max-age=3600');
    res.setHeader('Cross-Origin-Resource-Policy', 'cross-origin');
@@ -1666,7 +1613,7 @@ router.get('/:eventId/debug', adminAuth, requirePermission('photos.view'), requi
 router.post('/:eventId/chunked-upload/init', adminAuth, requirePermission('photos.upload'), requireEventOwnership, async (req, res) => {
  try {
    const { eventId } = req.params;
-    const { filename, fileSize, mimeType, totalChunks } = req.body;
+    const { filename, fileSize, totalChunks } = req.body;

    // Validate event exists
    const event = await db('events').where({ id: eventId }).first();
@@ -1675,8 +1622,23 @@ router.post('/:eventId/chunked-upload/init', adminAuth, requirePermission('photo
    }

    // Validate required fields
-    if (!filename || !fileSize || !mimeType) {
-      return res.status(400).json({ error: 'Missing required fields: filename, fileSize, mimeType' });
+    if (!filename || !fileSize) {
+      return res.status(400).json({ error: 'Missing required fields: filename, fileSize' });
+    }
+
+    // The client-declared mimeType is not trusted. It used to be stored on
+    // the photo row verbatim and echoed as Content-Type by the gallery
+    // routes, so a JPEG/HTML polyglot declared as text/html rendered inline
+    // on the app origin. The MIME is derived from the extension instead,
+    // and the extension has to be on the admin's allow-list, which is what
+    // the multipart path enforces through its multer fileFilter.
+    const ext = path.extname(String(filename)).slice(1).toLowerCase();
+    const mimeType = Object.prototype.hasOwnProperty.call(EXTENSION_TO_MIME, ext)
+      ? EXTENSION_TO_MIME[ext]
+      : null;
+    const allowedMimeTypes = await getAllowedMimeTypes();
+    if (!mimeType || !allowedMimeTypes.includes(mimeType)) {
+      return res.status(400).json({ error: 'File type not allowed' });
    }

    // Validate file size against the configured per-file cap. Hardcoding 10GB
@@ -10,6 +10,7 @@ const { parseBooleanInput } = require('../utils/parsers');
 const { getAppSetting } = require('../utils/appSettings');
 const archiver = require('archiver');
 const path = require('path');
+const { resolvePhotoContentType } = require('../utils/photoContentType');
 const router = express.Router();

 // #756: a NULL per-event hero_logo_visible means "inherit the global
@@ -1613,7 +1614,7 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,
    if (req.method === 'HEAD') {
      const headUseOriginal = await getUseOriginalFilenames();
      const headHeaders = {
-        'Content-Type': photo.mime_type || 'image/jpeg',
+        'Content-Type': resolvePhotoContentType(photo),
        'Content-Disposition': buildContentDisposition(pickRawDownloadName(photo, headUseOriginal)),
        'Accept-Ranges': 'bytes',
      };
@@ -1711,7 +1712,7 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,

    if (rendered) {
      res.set({
-        'Content-Type': photo.mime_type || 'image/jpeg',
+        'Content-Type': resolvePhotoContentType(photo),
        'Content-Disposition': contentDisposition,
        'Content-Length': rendered.length
      });
@@ -1769,7 +1770,7 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,

      const lastModified = stat.mtime ? new Date(stat.mtime).toUTCString() : null;
      const headers = {
-        'Content-Type': photo.mime_type || 'image/jpeg',
+        'Content-Type': resolvePhotoContentType(photo),
        'Content-Disposition': contentDisposition,
        'Accept-Ranges': 'bytes',
      };
@@ -1852,7 +1853,7 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,
    // their bytes on download. Set the header explicitly and stream the
    // file with res.sendFile-equivalent semantics.
    res.set({
-      'Content-Type': photo.mime_type || 'image/jpeg',
+      'Content-Type': resolvePhotoContentType(photo),
      'Content-Disposition': contentDisposition,
    });
    res.sendFile(filePath, (downloadError) => {
@@ -2603,7 +2604,7 @@ router.get('/:slug/photo/:photoId',
            'Content-Range': `bytes ${start}-${end}/${fileSize}`,
            'Accept-Ranges': 'bytes',
            'Content-Length': chunksize,
-            'Content-Type': photo.mime_type || 'video/mp4',
+            'Content-Type': resolvePhotoContentType(photo),
            'Cache-Control': 'private, max-age=1800',
            'X-Protection-Level': 'basic'
          });
@@ -2615,7 +2616,7 @@ router.get('/:slug/photo/:photoId',
        } else {
          res.writeHead(200, {
            'Content-Length': fileSize,
-            'Content-Type': photo.mime_type || 'video/mp4',
+            'Content-Type': resolvePhotoContentType(photo),
            'Accept-Ranges': 'bytes',
            'Cache-Control': 'private, max-age=1800',
            'X-Protection-Level': 'basic'
@@ -2659,7 +2660,7 @@ router.get('/:slug/photo/:photoId',
              const wmStat = await storage.stat(photo.watermark_path);
              if (wmStat) {
                res.set({
-                  'Content-Type': photo.mime_type || 'image/jpeg',
+                  'Content-Type': resolvePhotoContentType(photo),
                  'Content-Length': wmStat.size,
                  'Cache-Control': 'private, max-age=1800',
                  'ETag': etag,
@@ -2672,7 +2673,7 @@ router.get('/:slug/photo/:photoId',
              const watermarkFilePath = path.join(getStoragePath(), photo.watermark_path);
              if (fs.existsSync(watermarkFilePath)) {
                res.set({
-                  'Content-Type': photo.mime_type || 'image/jpeg',
+                  'Content-Type': resolvePhotoContentType(photo),
                  'Cache-Control': 'private, max-age=1800',
                  'ETag': etag,
                  'X-Protection-Level': 'basic'
@@ -2698,7 +2699,7 @@ router.get('/:slug/photo/:photoId',
          .catch(err => logger.warn(`Background watermark generation failed for photo ${photo.id}:`, err.message));

        res.set({
-          'Content-Type': photo.mime_type || 'image/jpeg',
+          'Content-Type': resolvePhotoContentType(photo),
          'Cache-Control': 'private, max-age=1800',
          'ETag': etag,
          'X-Protection-Level': 'basic'
@@ -2713,7 +2714,7 @@ router.get('/:slug/photo/:photoId',
        });
        if (useStorageBackend) {
          res.set('Content-Length', stat.size);
-          if (photo.mime_type) res.set('Content-Type', photo.mime_type);
+          res.set('Content-Type', resolvePhotoContentType(photo));
          const stream = await storage.get(storageKey);
          pipeStreamToResponse(stream, res, { context: `photo ${photo.id}` });
        } else {
@@ -1,4 +1,5 @@
 const express = require('express');
+const { resolvePhotoContentType } = require('../utils/photoContentType');
 const { db } = require('../database/db');
 const { formatBoolean } = require('../utils/dbCompat');
 const { verifyGalleryAccess } = require('../middleware/gallery');
@@ -170,7 +171,7 @@ router.get('/:slug/photo/:photoId/view', verifyGalleryAccess, blockHiddenGallery
    
    // Set security headers
    res.set({
-      'Content-Type': photo.mime_type || 'image/jpeg',
+      'Content-Type': resolvePhotoContentType(photo),
      'Content-Length': finalImage.length,
      'Cache-Control': 'private, no-cache, no-store, must-revalidate',
      'Pragma': 'no-cache',
@@ -351,7 +352,7 @@ router.get('/:slug/photo/:photoId/signed/:token', async (req, res) => {
    
    // Set appropriate headers
    res.set({
-      'Content-Type': photo.mime_type || 'image/jpeg',
+      'Content-Type': resolvePhotoContentType(photo),
      'Content-Length': imageBuffer.length,
      'Cache-Control': 'private, max-age=3600',
      'X-Content-Type-Options': 'nosniff'
@@ -1,4 +1,5 @@
 const express = require('express');
+const { resolvePhotoContentType } = require('../utils/photoContentType');
 const { db } = require('../database/db');
 const { verifyGalleryAccess, denySlideshowToken } = require('../middleware/gallery');
 const { blockHiddenGallery, bypassesReveal, isGalleryHidden } = require('../utils/revealMode');
@@ -246,7 +247,7 @@ router.get('/:slug/secure/:photoId/:token',

      // Set content type and security headers
      res.set({
-        'Content-Type': photo.mime_type || 'image/jpeg',
+        'Content-Type': resolvePhotoContentType(photo),
        'Content-Length': processedImage.length,
        'X-Protection-Level': protectionSettings.protectionLevel,
        'X-Remaining-Uses': tokenValidation.remaining
@@ -436,7 +437,7 @@ router.get('/:slug/secure-download/:photoId/:token',
      const downloadName = pickRawDownloadName(photo, useOriginal);

      res.set({
-        'Content-Type': photo.mime_type || 'image/jpeg',
+        'Content-Type': resolvePhotoContentType(photo),
        'Content-Disposition': buildContentDisposition(downloadName),
        'Content-Length': fileBuffer.length,
        'X-Download-Protected': 'true'
@@ -0,0 +1,49 @@
+/**
+ * Content-Type for a served photo row. Invariant: the header is ALWAYS
+ * image/* or video/*, never the stored value verbatim.
+ *
+ * photos.mime_type is client-influenced: the chunked-upload path used to
+ * store whatever MIME the browser (or a crafted request) declared, and the
+ * S3 auto-importer stores whatever mime-types derives. Echoing it inline
+ * under the app origin turned a JPEG/HTML polyglot with mime_type text/html
+ * into stored HTML injection for every gallery guest. The admin photo route
+ * (#908 + external review) already resolved this properly; this is that
+ * logic, shared so every serving route applies the same rule.
+ *
+ *  - Images ignore the stored value unless it is a header-safe raster type:
+ *    migration 039 backfilled image/jpeg onto every legacy row (PNGs
+ *    included), so the extension is the more trustworthy signal, normalised
+ *    via the shared map, jpeg fallback when unknown. The scriptable svg /
+ *    *+xml family is never honoured.
+ *  - Videos prefer a stored video/ type, then the extension map (.mov ->
+ *    video/quicktime, .webm -> video/webm, ...), then video/mp4.
+ *  - Full-token validation, not a prefix check: header-invalid characters
+ *    (video/mp4\r\nX: y) would make setHeader throw -- a permanent 500 for
+ *    that photo instead of a safe fallback.
+ */
+const path = require('path');
+const { EXTENSION_TO_MIME } = require('../services/uploadSettings');
+
+function resolvePhotoContentType(photo) {
+  const ext = path.extname(photo?.filename || '').slice(1).toLowerCase();
+  // Own-property lookup: a client-controlled filename ending in .constructor
+  // / .__proto__ would otherwise return an inherited Object.prototype member.
+  const extMime = Object.prototype.hasOwnProperty.call(EXTENSION_TO_MIME, ext)
+    ? EXTENSION_TO_MIME[ext]
+    : null;
+  const stored = typeof photo?.mime_type === 'string' ? photo.mime_type : '';
+  const storedVideoMime = /^video\/[\w.+-]+$/.test(stored) ? stored : null;
+  const storedImageMime =
+    /^image\/[\w.+-]+$/.test(stored) && !/^image\/svg|xml/i.test(stored)
+      ? stored
+      : null;
+  const isVideo = photo?.media_type === 'video' ||
+    Boolean(storedVideoMime) ||
+    Boolean(extMime && extMime.startsWith('video/'));
+
+  return isVideo
+    ? storedVideoMime || (extMime && extMime.startsWith('video/') ? extMime : null) || 'video/mp4'
+    : (extMime && extMime.startsWith('image/') ? extMime : null) || storedImageMime || 'image/jpeg';
+}
+
+module.exports = { resolvePhotoContentType };