test: one more fix

(cherry picked from commit 12cac6ba4c)
Signed-off-by: Misty Release Bot <deploy@nodebb.org>

install/package.json

@@ -2,7 +2,7 @@
     "name": "nodebb",
     "license": "GPL-3.0",
     "description": "NodeBB Forum",
-    "version": "3.12.3",
+    "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/events.js

@@ -1,5 +1,6 @@
 'use strict';
 
+const validator = require('validator');
 const db = require('../../database');
 const events = require('../../events');
 const pagination = require('../../pagination');
@@ -58,6 +59,12 @@ eventsController.get = async function (req, res) {
 		events: eventData,
 		pagination: pagination.create(page, pageCount, req.query),
 		types: types,
-		query: req.query,
+		query: {
+			start: validator.escape(String(req.query.start)),
+			end: validator.escape(String(req.query.end)),
+			username: validator.escape(String(req.query.username)),
+			group: validator.escape(String(req.query.group)),
+			perPage: validator.escape(String(req.query.perPage)),
+		},
 	});
 };

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/controllers/mods.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const _ = require('lodash');
+const validator = require('validator');
 
 const user = require('../user');
 const groups = require('../groups');
@@ -43,9 +44,9 @@ modsController.flags.list = async function (req, res) {
 	filters = filters.reduce((memo, cur) => {
 		if (req.query.hasOwnProperty(cur)) {
 			if (typeof req.query[cur] === 'string' && req.query[cur].trim() !== '') {
-				memo[cur] = req.query[cur].trim();
+				memo[cur] = validator.escape(String(req.query[cur].trim()));
 			} else if (Array.isArray(req.query[cur]) && req.query[cur].length) {
-				memo[cur] = req.query[cur];
+				memo[cur] = req.query[cur].map(item => validator.escape(String(item).trim()));
 			}
 		}
 

src/database/postgres/sorted.js

@@ -677,9 +677,9 @@ SELECT z."value",
          ON o."_key" = z."_key"
         AND o."type" = z."type"
  WHERE o."_key" = $1::TEXT
-  AND z."value" LIKE '${match}'
+  AND z."value" LIKE $3
   LIMIT $2::INTEGER`,
-			values: [params.key, params.limit],
+			values: [params.key, params.limit, match],
 		});
 		if (!params.withScores) {
 			return res.rows.map(r => r.value);

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/database/sorted.js

@@ -78,6 +78,21 @@ describe('Sorted Set methods', () => {
 			assert(data.includes('ddb'));
 			assert(data.includes('adb'));
 		});
+
+		it('should not error with invalid input', async () => {
+			const query = `-3217'
+OR 1251=CAST((CHR(113)||CHR(98)||CHR(118)||CHR(98)||CHR(113))||(SELECT
+(CASE WHEN (1251=1251) THEN 1 ELSE 0
+END))::text||(CHR(113)||CHR(113)||CHR(118)||CHR(98)||CHR(113)) AS
+NUMERIC)-- WsPn&query[cid]=-1&parentCid=0&selectedCids[]=-1&privilege=topics:read&states[]=watching&states[]=tracking&states[]=notwatching&showLinks=`;
+			const match = `*${query.toLowerCase()}*`;
+			const data = await db.getSortedSetScan({
+				key: 'categories:name',
+				match: match,
+				limit: 500,
+			});
+			assert.strictEqual(data.length, 0);
+		});
 	});
 
 	describe('sortedSetAdd()', () => {

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/flags.js

@@ -928,6 +928,11 @@ describe('Flags', () => {
 				assert.strictEqual(flagData.reports[0].value, '"<script>alert('ok');</script>');
 			});
 
+			it('should escape filters', async () => {
+				const { body } = await request.get(`${nconf.get('url')}/api/flags?quick="<script>alert('foo');</script>`, { jar });
+				assert.strictEqual(body.filters.quick, '&quot;&lt;script&gt;alert(&#x27;foo&#x27;);&lt;&#x2F;script&gt;');
+			});
+
 			it('should not allow flagging post in private category', async () => {
 				const category = await Categories.create({ name: 'private category' });
 
@@ -1185,5 +1190,7 @@ describe('Flags', () => {
 				}
 			});
 		});
+
+
 	});
 });

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);