diff --git a/src/controllers/groups.js b/src/controllers/groups.js index cdfb0ef6c6..d618cbf858 100644 --- a/src/controllers/groups.js +++ b/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 }]); diff --git a/src/groups/cover.js b/src/groups/cover.js index c4b4024486..b571043c11 100644 --- a/src/groups/cover.js +++ b/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, diff --git a/src/user/picture.js b/src/user/picture.js index 19c05b8788..bc267c1875 100644 --- a/src/user/picture.js +++ b/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) { - 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 + ']]'); - } + validateUpload(data, meta.config.maximumCoverImageSize); - if (data.file) { - picture.path = data.file.path; - } else { - picture.path = await image.writeImageDataToTempFile(data.imageData); - } + picture.path = await getTempPath(data); - 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); - const filename = generateProfileImageFilename(data.uid, 'profilecover', extension); + const extension = file.typeToExtension(getMimeType(data)); + const filename = 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) { - throw new Error('[[error:invalid-data]]'); - } + validateUpload(data, meta.config.maximumProfileImageSize); - const size = data.file ? data.file.size : image.sizeFromBase64(data.imageData); - 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); + const extension = file.typeToExtension(getMimeType(data)); if (!extension) { throw new Error('[[error:invalid-image-extension]]'); } - if (data.file) { - picture.path = data.file.path; - } else { - picture.path = await image.writeImageDataToTempFile(data.imageData); - } - + picture.path = await getTempPath(data); 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) { diff --git a/test/groups.js b/test/groups.js index 103a9cd3d7..c6ca8b1568 100644 --- a/test/groups.js +++ b/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', diff --git a/test/uploads.js b/test/uploads.js index 578b99515d..6d05046be1 100644 --- a/test/uploads.js +++ b/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;