diff --git a/src/controllers/admin/uploads.js b/src/controllers/admin/uploads.js index 74d1370de0..c8e8075456 100644 --- a/src/controllers/admin/uploads.js +++ b/src/controllers/admin/uploads.js @@ -4,7 +4,6 @@ const path = require('path'); const nconf = require('nconf'); const fs = require('fs'); const winston = require('winston'); -const sanitizeHtml = require('sanitize-html'); const meta = require('../../meta'); const posts = require('../../posts'); @@ -159,50 +158,11 @@ uploadsController.uploadCategoryPicture = async function (req, res, next) { return next(new Error('[[error:invalid-json]]')); } - if (uploadedFile.path.endsWith('.svg')) { - await sanitizeSvg(uploadedFile.path); - } - await validateUpload(uploadedFile, allowedImageTypes); const filename = `category-${params.cid}${path.extname(uploadedFile.name)}`; await uploadImage(filename, 'category', uploadedFile, req, res, next); }; -async function sanitizeSvg(filePath) { - const dirty = await fs.promises.readFile(filePath, 'utf8'); - const clean = sanitizeHtml(dirty, { - allowedTags: [ - 'svg', 'g', 'defs', 'linearGradient', 'radialGradient', 'stop', - 'circle', 'ellipse', 'polygon', 'polyline', 'path', 'rect', - 'line', 'text', 'tspan', 'use', 'symbol', 'clipPath', 'mask', 'pattern', - 'filter', 'feGaussianBlur', 'feOffset', 'feBlend', 'feColorMatrix', 'feMerge', 'feMergeNode', - ], - allowedAttributes: { - '*': [ - // Geometry - 'x', 'y', 'x1', 'x2', 'y1', 'y2', 'cx', 'cy', 'r', 'rx', 'ry', - 'width', 'height', 'd', 'points', 'viewBox', 'transform', - - // Presentation - 'fill', 'stroke', 'stroke-width', 'opacity', - 'stop-color', 'stop-opacity', 'offset', 'style', 'class', - - // Text - 'text-anchor', 'font-size', 'font-family', - - // Misc - 'id', 'clip-path', 'mask', 'filter', 'gradientUnits', 'gradientTransform', - 'xmlns', 'preserveAspectRatio', - ], - }, - parser: { - lowerCaseTags: false, - lowerCaseAttributeNames: false, - }, - }); - await fs.promises.writeFile(filePath, clean); -} - uploadsController.uploadFavicon = async function (req, res, next) { const uploadedFile = req.files[0]; const allowedTypes = ['image/x-icon', 'image/vnd.microsoft.icon']; @@ -298,10 +258,6 @@ uploadsController.uploadOgImage = async function (req, res, next) { async function upload(name, req, res, next) { const uploadedFile = req.files[0]; - if (uploadedFile.path.endsWith('.svg')) { - await sanitizeSvg(uploadedFile.path); - } - await validateUpload(uploadedFile, allowedImageTypes); const filename = name + path.extname(uploadedFile.name); await uploadImage(filename, 'system', uploadedFile, req, res, next); diff --git a/src/file.js b/src/file.js index 639cc9f58c..4c3c911950 100644 --- a/src/file.js +++ b/src/file.js @@ -7,6 +7,7 @@ const winston = require('winston'); const { mkdirp } = require('mkdirp'); const mime = require('mime'); const graceful = require('graceful-fs'); +const sanitizeHtml = require('sanitize-html'); const slugify = require('./slugify'); @@ -27,6 +28,10 @@ file.saveFileToLocal = async function (filename, folder, tempPath) { winston.verbose(`Saving file ${filename} to : ${uploadPath}`); await mkdirp(path.dirname(uploadPath)); + if (tempPath.endsWith('.svg')) { + await sanitizeSvg(tempPath); + } + await fs.promises.copyFile(tempPath, uploadPath); return { url: `/assets/uploads/${folder ? `${folder}/` : ''}${filename}`, @@ -155,4 +160,39 @@ file.walk = async function (dir) { return files.reduce((a, f) => a.concat(f), []); }; +async function sanitizeSvg(filePath) { + const dirty = await fs.promises.readFile(filePath, 'utf8'); + const clean = sanitizeHtml(dirty, { + allowedTags: [ + 'svg', 'g', 'defs', 'linearGradient', 'radialGradient', 'stop', + 'circle', 'ellipse', 'polygon', 'polyline', 'path', 'rect', + 'line', 'text', 'tspan', 'use', 'symbol', 'clipPath', 'mask', 'pattern', + 'filter', 'feGaussianBlur', 'feOffset', 'feBlend', 'feColorMatrix', 'feMerge', 'feMergeNode', + ], + allowedAttributes: { + '*': [ + // Geometry + 'x', 'y', 'x1', 'x2', 'y1', 'y2', 'cx', 'cy', 'r', 'rx', 'ry', + 'width', 'height', 'd', 'points', 'viewBox', 'transform', + + // Presentation + 'fill', 'stroke', 'stroke-width', 'opacity', + 'stop-color', 'stop-opacity', 'offset', 'style', 'class', + + // Text + 'text-anchor', 'font-size', 'font-family', + + // Misc + 'id', 'clip-path', 'mask', 'filter', 'gradientUnits', 'gradientTransform', + 'xmlns', 'preserveAspectRatio', + ], + }, + parser: { + lowerCaseTags: false, + lowerCaseAttributeNames: false, + }, + }); + await fs.promises.writeFile(filePath, clean); +} + require('./promisify')(file); diff --git a/test/files/dirty.svg b/test/files/dirty.svg new file mode 100644 index 0000000000..a948be59b2 --- /dev/null +++ b/test/files/dirty.svg @@ -0,0 +1,4 @@ + + + + \ No newline at end of file diff --git a/test/uploads.js b/test/uploads.js index 3b361e36d3..4fc0e41a64 100644 --- a/test/uploads.js +++ b/test/uploads.js @@ -338,6 +338,15 @@ describe('Upload Controllers', () => { assert.equal(body[0].url, `${nconf.get('relative_path')}/assets/uploads/category/category-1.png`); }); + it('should upload svg as category image after cleaning it up', async () => { + const { response, body } = await helpers.uploadFile(`${nconf.get('url')}/api/admin/category/uploadpicture`, path.join(__dirname, '../test/files/dirty.svg'), { params: JSON.stringify({ cid: cid }) }, jar, csrf_token); + assert.equal(response.statusCode, 200); + assert(Array.isArray(body)); + assert.equal(body[0].url, `${nconf.get('relative_path')}/assets/uploads/category/category-1.svg`); + const svgContents = await fs.readFile(path.join(__dirname, '../test/uploads/category/category-1.svg'), 'utf-8'); + assert.strictEqual(svgContents.includes('