test: one more fix

install/package.json

@@ -2,7 +2,7 @@
     "name": "nodebb",
     "license": "GPL-3.0",
     "description": "NodeBB Forum",
-    "version": "3.12.6",
+    "version": "3.12.7",
     "homepage": "https://www.nodebb.org",
     "repository": {
         "type": "git",

public/openapi/write/posts/pid/diffs.yaml

@@ -24,6 +24,10 @@ get:
               response:
                 type: object
                 properties:
+                  uid:
+                    type: number
+                  pid:
+                    type: number
                   timestamps:
                     type: array
                     items:
@@ -37,6 +41,8 @@ get:
                           type: string
                         username:
                           type: string
+                        uid:
+                          type: number
                   editable:
                     type: boolean
                   deletable:

src/api/posts.js

@@ -427,10 +427,12 @@ async function diffsPrivilegeCheck(pid, uid) {
 
 postsAPI.getDiffs = async (caller, data) => {
 	await diffsPrivilegeCheck(data.pid, caller.uid);
-	const timestamps = await posts.diffs.list(data.pid);
-	const post = await posts.getPostFields(data.pid, ['timestamp', 'uid']);
+	const [timestamps, post, diffs] = await Promise.all([
+		posts.diffs.list(data.pid),
+		posts.getPostFields(data.pid, ['timestamp', 'uid']),
+		posts.diffs.get(data.pid),
+	]);
 
-	const diffs = await posts.diffs.get(data.pid);
 	const uids = diffs.map(diff => diff.uid || null);
 	uids.push(post.uid);
 	let usernames = await user.getUsersFields(uids, ['username']);
@@ -444,18 +446,21 @@ postsAPI.getDiffs = async (caller, data) => {
 
 	// timestamps returned by posts.diffs.list are strings
 	timestamps.push(String(post.timestamp));
-
-	return {
+	const result = await plugins.hooks.fire('filter:post.getDiffs', {
+		uid: caller.uid,
+		pid: data.pid,
 		timestamps: timestamps,
 		revisions: timestamps.map((timestamp, idx) => ({
 			timestamp: timestamp,
 			username: usernames[idx],
+			uid: uids[idx],
 		})),
 		// Only admins, global mods and moderator of that cid can delete a diff
 		deletable: isAdmin || isModerator,
 		// These and post owners can restore to a different post version
 		editable: isAdmin || isModerator || parseInt(caller.uid, 10) === parseInt(post.uid, 10),
-	};
+	});
+	return result;
 };
 
 postsAPI.loadDiff = async (caller, data) => {

src/controllers/admin/uploads.js

@@ -183,10 +183,6 @@ uploadsController.uploadMaskableIcon = async function (req, res, next) {
 	}
 };
 
-uploadsController.uploadLogo = async function (req, res, next) {
-	await upload('site-logo', req, res, next);
-};
-
 uploadsController.uploadFile = async function (req, res, next) {
 	const uploadedFile = req.files.files[0];
 	let params;
@@ -207,6 +203,10 @@ uploadsController.uploadFile = async function (req, res, next) {
 	}
 };
 
+uploadsController.uploadLogo = async function (req, res, next) {
+	await upload('site-logo', req, res, next);
+};
+
 uploadsController.uploadDefaultAvatar = async function (req, res, next) {
 	await upload('avatar-default', req, res, next);
 };

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 (filename.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);

test/files/dirty.svg

@@ -0,0 +1,4 @@
+<svg width="100" height="100" xmlns="http://www.w3.org/2000/svg">
+	<rect x="10" y="10" width="80" height="80" fill="red" stroke="black" stroke-width="4"/>
+</svg>
+<script>alert('foo');</script>

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('<script>'), false);
+		});
+
 		it('should upload default avatar', async () => {
 			const { response, body } = await helpers.uploadFile(`${nconf.get('url')}/api/admin/uploadDefaultAvatar`, path.join(__dirname, '../test/files/test.png'), { }, jar, csrf_token);
 			assert.equal(response.statusCode, 200);