Force download for SVG

Prevent XSS when the file is opened directly

packages/tests/src/api/server/config.ts

@@ -953,6 +953,37 @@ describe('Test config', function () {
         })
       })
 
+      describe('SVG', function () {
+        let svgUrl: string
+
+        it('Should update instance favicon with an SVG', async function () {
+          await server.config.updateInstanceLogo({ type: 'favicon', fixture: 'peertube.svg' })
+
+          const htmlConfig = await server.config.getConfig()
+
+          const favicons = htmlConfig.instance.logo.filter(l => l.type === 'favicon')
+          expect(favicons).to.have.lengthOf(1)
+          expect(favicons[0].isFallback).to.be.false
+
+          svgUrl = favicons[0].fileUrl
+          expect(svgUrl).to.match(/\.svg$/)
+
+          await testFileExistsOnFSOrNot(server, 'uploads/images', basename(svgUrl), true)
+        })
+
+        it('Should serve the SVG with a Content-Disposition attachment header', async function () {
+          const res = await makeRawRequest({ url: svgUrl, expectedStatus: HttpStatusCode.OK_200 })
+
+          expect(res.headers['content-disposition']).to.equal('attachment')
+        })
+
+        it('Should remove the SVG favicon', async function () {
+          await server.config.deleteInstanceLogo({ type: 'favicon' })
+
+          await testFileExistsOnFSOrNot(server, 'uploads/images', basename(svgUrl), false)
+        })
+      })
+
       describe('Default logo', function () {
         before(async function () {
           await server.config.updateInstanceImage({ type: ActorImageType.AVATAR, fixture: 'avatar.png' })

packages/tests/src/server-helpers/image.ts

@@ -6,6 +6,7 @@ import { processImage, processSVG } from '@peertube/peertube-server/core/helpers
 import { expect } from 'chai'
 import { ensureDir, remove } from 'fs-extra/esm'
 import { readFile, writeFile } from 'fs/promises'
+import { tmpdir } from 'os'
 import { join } from 'path'
 import sharp from 'sharp'
 
@@ -123,7 +124,7 @@ describe('Image helpers', function () {
 })
 
 describe('SVG sanitization', function () {
-  const svgDir = join(root(), 'test-svg')
+  const svgDir = join(tmpdir(), 'test-svg')
   const svgSrc = join(svgDir, 'input.svg')
   const svgDest = join(svgDir, 'output.svg')
 

server/core/controllers/static.ts

@@ -103,7 +103,15 @@ staticRouter.use(
 
 staticRouter.use(
   STATIC_PATHS.UPLOAD_IMAGES,
-  express.static(DIRECTORIES.UPLOAD_IMAGES, { fallthrough: false }),
+  express.static(DIRECTORIES.UPLOAD_IMAGES, {
+    fallthrough: false,
+    setHeaders: (res, filePath) => {
+      // Force a download instead of inline rendering to prevent XSS if the svg is opened directly
+      if (filePath.endsWith('.svg')) {
+        res.setHeader('Content-Disposition', 'attachment')
+      }
+    }
+  }),
   handleStaticError
 )