fix: only allow png/jpg/bmp in cover/profile images

src/controllers/groups.js

@@ -111,7 +111,7 @@ groupsController.members = async function (req, res, next) {
 };
 
 groupsController.uploadCover = async function (req, res, next) {
-	var params = JSON.parse(req.body.params);
+	const params = JSON.parse(req.body.params);
 
 	try {
 		const isOwner = await groups.ownership.isOwner(req.uid, params.groupName);
@@ -119,7 +119,7 @@ groupsController.uploadCover = async function (req, res, next) {
 			throw new Error('[[error:no-privileges]]');
 		}
 		const image = await groups.updateCover(req.uid, {
-			file: req.files.files[0].path,
+			file: req.files.files[0],
 			groupName: params.groupName,
 		});
 		res.json([{ url: image.url }]);

src/groups/cover.js

@@ -7,6 +7,7 @@ const image = require('../image');
 const file = require('../file');
 
 module.exports = function (Groups) {
+	const allowedTypes = ['image/png', 'image/jpeg', 'image/bmp'];
 	Groups.updateCoverPosition = async function (groupName, position) {
 		if (!groupName) {
 			throw new Error('[[error:invalid-data]]');
@@ -15,15 +16,21 @@ module.exports = function (Groups) {
 	};
 
 	Groups.updateCover = async function (uid, data) {
-		let tempPath = data.file ? data.file : '';
+		let tempPath = data.file ? data.file.path : '';
 		try {
 			// Position only? That's fine
 			if (!data.imageData && !data.file && data.position) {
 				return await Groups.updateCoverPosition(data.groupName, data.position);
 			}
+			const type = data.file ? data.file.type : image.mimeFromBase64(data.imageData);
+			if (!type || !allowedTypes.includes(type)) {
+				throw new Error('[[error:invalid-image]]');
+			}
+
 			if (!tempPath) {
 				tempPath = await image.writeImageDataToTempFile(data.imageData);
 			}
+
 			const filename = 'groupCover-' + data.groupName + path.extname(tempPath);
 			const uploadData = await image.uploadImage(filename, 'files', {
 				path: tempPath,

src/user/picture.js

@@ -8,6 +8,7 @@ const image = require('../image');
 const meta = require('../meta');
 
 module.exports = function (User) {
+	const allowedTypes = ['image/png', 'image/jpeg', 'image/bmp'];
 	User.updateCoverPosition = async function (uid, position) {
 		// Reject anything that isn't two percentages
 		if (!/^[\d.]+%\s[\d.]+%$/.test(position)) {
@@ -29,27 +30,12 @@ module.exports = function (User) {
 				return await User.updateCoverPosition(data.uid, data.position);
 			}
 
-			if (!data.imageData && !data.file) {
+			validateUpload(data, meta.config.maximumCoverImageSize);
-				throw new Error('[[error:invalid-data]]');
-			}
-			const size = data.file ? data.file.size : image.sizeFromBase64(data.imageData);
-			if (size > meta.config.maximumCoverImageSize * 1024) {
-				throw new Error('[[error:file-too-big, ' + meta.config.maximumCoverImageSize + ']]');
-			}
 
-			if (data.file) {
+			picture.path = await getTempPath(data);
-				picture.path = data.file.path;
-			} else {
-				picture.path = await image.writeImageDataToTempFile(data.imageData);
-			}
 
-			const type = data.file ? data.file.type : image.mimeFromBase64(data.imageData);
+			const extension = file.typeToExtension(getMimeType(data));
-			if (!type || !type.match(/^image./)) {
+			const filename = data.uid + '-profilecover' + extension;
-				throw new Error('[[error:invalid-image]]');
-			}
-
-			const extension = file.typeToExtension(type);
-			const filename = generateProfileImageFilename(data.uid, 'profilecover', extension);
 			const uploadData = await image.uploadImage(filename, 'profile', picture);
 
 			await User.setUserField(data.uid, 'cover:url', uploadData.url);
@@ -77,31 +63,14 @@ module.exports = function (User) {
 				throw new Error('[[error:profile-image-uploads-disabled]]');
 			}
 
-			if (!data.imageData && !data.file) {
+			validateUpload(data, meta.config.maximumProfileImageSize);
-				throw new Error('[[error:invalid-data]]');
-			}
 
-			const size = data.file ? data.file.size : image.sizeFromBase64(data.imageData);
+			const extension = file.typeToExtension(getMimeType(data));
-			const uploadSize = meta.config.maximumProfileImageSize;
-			if (size > uploadSize * 1024) {
-				throw new Error('[[error:file-too-big, ' + uploadSize + ']]');
-			}
-
-			const type = data.file ? data.file.type : image.mimeFromBase64(data.imageData);
-			if (!type || !type.match(/^image./)) {
-				throw new Error('[[error:invalid-image]]');
-			}
-			const extension = file.typeToExtension(type);
 			if (!extension) {
 				throw new Error('[[error:invalid-image-extension]]');
 			}
 
-			if (data.file) {
+			picture.path = await getTempPath(data);
-				picture.path = data.file.path;
-			} else {
-				picture.path = await image.writeImageDataToTempFile(data.imageData);
-			}
-
 			picture.path = await convertToPNG(picture.path, extension);
 
 			await image.resizeImage({
@@ -110,7 +79,7 @@ module.exports = function (User) {
 				height: meta.config.profileImageDimension,
 			});
 
-			const filename = generateProfileImageFilename(data.uid, 'profileavatar', extension);
+			const filename = generateProfileImageFilename(data.uid, extension);
 			const uploadedImage = await image.uploadImage(filename, 'profile', picture);
 
 			await User.setUserFields(data.uid, {
@@ -123,6 +92,32 @@ module.exports = function (User) {
 		}
 	};
 
+	function validateUpload(data, maxSize) {
+		if (!data.imageData && !data.file) {
+			throw new Error('[[error:invalid-data]]');
+		}
+		const size = data.file ? data.file.size : image.sizeFromBase64(data.imageData);
+		if (size > maxSize * 1024) {
+			throw new Error('[[error:file-too-big, ' + maxSize + ']]');
+		}
+
+		const type = getMimeType(data);
+		if (!type || !allowedTypes.includes(type)) {
+			throw new Error('[[error:invalid-image]]');
+		}
+	}
+
+	function getMimeType(data) {
+		return data.file ? data.file.type : image.mimeFromBase64(data.imageData);
+	}
+
+	async function getTempPath(data) {
+		if (data.file) {
+			return data.file.path;
+		}
+		return await image.writeImageDataToTempFile(data.imageData);
+	}
+
 	async function convertToPNG(path, extension) {
 		const convertToPNG = meta.config['profile:convertProfileImageToPNG'] === 1;
 		if (!convertToPNG) {
@@ -133,10 +128,10 @@ module.exports = function (User) {
 		return newPath;
 	}
 
-	function generateProfileImageFilename(uid, type, extension) {
+	function generateProfileImageFilename(uid, extension) {
 		const keepAllVersions = meta.config['profile:keepAllUserImages'] === 1;
 		const convertToPNG = meta.config['profile:convertProfileImageToPNG'] === 1;
-		return uid + '-' + type + (keepAllVersions ? '-' + Date.now() : '') + (convertToPNG ? '.png' : extension);
+		return uid + '-profileavatar' + (keepAllVersions ? '-' + Date.now() : '') + (convertToPNG ? '.png' : extension);
 	}
 
 	User.removeCoverPicture = async function (data) {

test/groups.js

@@ -1296,7 +1296,10 @@ describe('Groups', function () {
 		it('should upload group cover image from file', function (done) {
 			var data = {
 				groupName: 'Test',
-				file: imagePath,
+				file: {
+					path: imagePath,
+					type: 'image/png',
+				},
 			};
 			socketGroups.cover.update({ uid: adminUid }, data, function (err, data) {
 				assert.ifError(err);
@@ -1328,6 +1331,17 @@ describe('Groups', function () {
 			});
 		});
 
+		it('should fail to upload group cover with invalid image', function (done) {
+			var data = {
+				groupName: 'Test',
+				imageData: 'data:image/svg;base64,iVBORw0KGgoAAAANSUhEUgAAABwA',
+			};
+			socketGroups.cover.update({ uid: adminUid }, data, function (err, data) {
+				assert.equal(err.message, '[[error:invalid-image]]');
+				done();
+			});
+		});
+
 		it('should update group cover position', function (done) {
 			var data = {
 				groupName: 'Test',

test/uploads.js

@@ -215,6 +215,13 @@ describe('Upload Controllers', function () {
 			});
 		});
 
+		it('should not allow svg uploads', function (done) {
+			socketUser.updateCover({ uid: 1 }, { uid: 1, imageData: 'data:image/svg;base64,PHN2Zy9vbmxvYWQ9YWxlcnQoMik+' }, function (err) {
+				assert.equal(err.message, '[[error:invalid-image]]');
+				done();
+			});
+		});
+
 		it('should not allow non image uploads', function (done) {
 			socketUser.uploadCroppedPicture({ uid: 1 }, { uid: 1, imageData: 'data:text/html;base64,PHN2Zy9vbmxvYWQ9YWxlcnQoMik+' }, function (err) {
 				assert.equal(err.message, '[[error:invalid-image]]');
@@ -222,6 +229,13 @@ describe('Upload Controllers', function () {
 			});
 		});
 
+		it('should not allow svg uploads', function (done) {
+			socketUser.uploadCroppedPicture({ uid: 1 }, { uid: 1, imageData: 'data:image/svg;base64,PHN2Zy9vbmxvYWQ9YWxlcnQoMik+' }, function (err) {
+				assert.equal(err.message, '[[error:invalid-image]]');
+				done();
+			});
+		});
+
 		it('should delete users uploads if account is deleted', function (done) {
 			var jar;
 			var uid;