diff --git a/public/src/client/account/edit.js b/public/src/client/account/edit.js index a546c71e6f..c6f99d733b 100644 --- a/public/src/client/account/edit.js +++ b/public/src/client/account/edit.js @@ -228,7 +228,6 @@ define('forum/account/edit', ['forum/account/header', 'translator', 'components' pictureCropper.show({ socketMethod: 'user.uploadCroppedPicture', - route: config.relative_path + '/api/user/' + ajaxify.data.userslug + '/uploadpicture', aspectRatio: 1 / 1, paramName: 'uid', paramValue: ajaxify.data.theirid, diff --git a/src/file.js b/src/file.js index 565f7b7f27..cb384ec64a 100644 --- a/src/file.js +++ b/src/file.js @@ -30,6 +30,9 @@ file.saveFileToLocal = async function (filename, folder, tempPath) { filename = filename.split('.').map(name => utils.slugify(name)).join('.'); const uploadPath = path.join(nconf.get('upload_path'), folder, filename); + if (!uploadPath.startsWith(nconf.get('upload_path'))) { + throw new Error('[[error:invalid-path]]'); + } winston.verbose('Saving file ' + filename + ' to : ' + uploadPath); await mkdirp(path.dirname(uploadPath)); diff --git a/src/image.js b/src/image.js index 4bf588621c..e7e4652fed 100644 --- a/src/image.js +++ b/src/image.js @@ -101,7 +101,7 @@ image.stripEXIF = async function (path) { const sharp = requireSharp(); await sharp(buffer, { failOnError: true }).rotate().toFile(path); } catch (err) { - winston.error(err); + winston.error(err.stack); } }; diff --git a/src/user/picture.js b/src/user/picture.js index 2d6811b99e..5419b062e5 100644 --- a/src/user/picture.js +++ b/src/user/picture.js @@ -45,9 +45,9 @@ module.exports = function (User) { validateUpload(data, meta.config.maximumCoverImageSize, ['image/png', 'image/jpeg', 'image/bmp']); - picture.path = await getTempPath(data); + picture.path = await image.writeImageDataToTempFile(data.imageData); - const extension = file.typeToExtension(getMimeType(data)); + const extension = file.typeToExtension(image.mimeFromBase64(data.imageData)); const filename = data.uid + '-profilecover' + extension; const uploadData = await image.uploadImage(filename, 'profile', picture); @@ -61,7 +61,7 @@ module.exports = function (User) { url: uploadData.url, }; } finally { - file.delete(picture.path || (data.file && data.file.path)); + file.delete(picture.path); } }; @@ -78,12 +78,12 @@ module.exports = function (User) { validateUpload(data, meta.config.maximumProfileImageSize, User.getAllowedImageTypes()); - const extension = file.typeToExtension(getMimeType(data)); + const extension = file.typeToExtension(image.mimeFromBase64(data.imageData)); if (!extension) { throw new Error('[[error:invalid-image-extension]]'); } - picture.path = await getTempPath(data); + picture.path = await image.writeImageDataToTempFile(data.imageData); picture.path = await convertToPNG(picture.path); await image.resizeImage({ @@ -101,36 +101,25 @@ module.exports = function (User) { }); return uploadedImage; } finally { - file.delete(picture.path || (data.file && data.file.path)); + file.delete(picture.path); } }; function validateUpload(data, maxSize, allowedTypes) { - if (!data.imageData && !data.file) { + if (!data.imageData) { throw new Error('[[error:invalid-data]]'); } - const size = data.file ? data.file.size : image.sizeFromBase64(data.imageData); + const size = image.sizeFromBase64(data.imageData); if (size > maxSize * 1024) { throw new Error('[[error:file-too-big, ' + maxSize + ']]'); } - const type = getMimeType(data); + const type = image.mimeFromBase64(data.imageData); 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) { const convertToPNG = meta.config['profile:convertProfileImageToPNG'] === 1; if (!convertToPNG) { diff --git a/test/file.js b/test/file.js index 759bc55ac5..24aa655bed 100644 --- a/test/file.js +++ b/test/file.js @@ -96,6 +96,14 @@ describe('file', function () { done(); }); }); + + it('should error if folder is relative', function (done) { + file.saveFileToLocal(filename, '../../text', tempPath + '000000000', function (err) { + assert(err); + assert.strictEqual(err.message, '[[error:invalid-path]]'); + done(); + }); + }); }); it('should walk directory', function (done) { diff --git a/test/helpers/index.js b/test/helpers/index.js index 554ffb813c..f38ef8795c 100644 --- a/test/helpers/index.js +++ b/test/helpers/index.js @@ -107,7 +107,7 @@ helpers.uploadFile = function (uploadEndPoint, filePath, body, jar, csrf_token, return callback(err); } if (res.statusCode !== 200) { - winston.error(body); + winston.error(JSON.stringify(body)); } callback(null, res, body); }); diff --git a/test/uploads.js b/test/uploads.js index 1fdef50ca4..ec8618f3f6 100644 --- a/test/uploads.js +++ b/test/uploads.js @@ -220,6 +220,13 @@ describe('Upload Controllers', function () { }); }); + it('should not allow non image uploads', function (done) { + socketUser.updateCover({ uid: 1 }, { uid: 1, file: { path: '../../text.txt' } }, function (err) { + assert.equal(err.message, '[[error:invalid-data]]'); + done(); + }); + }); + it('should not allow non image uploads', function (done) { socketUser.updateCover({ uid: 1 }, { uid: 1, imageData: 'data:text/html;base64,PHN2Zy9vbmxvYWQ9YWxlcnQoMik+' }, function (err) { assert.equal(err.message, '[[error:invalid-image]]'); @@ -234,6 +241,13 @@ describe('Upload Controllers', function () { }); }); + it('should not allow non image uploads', function (done) { + socketUser.uploadCroppedPicture({ uid: 1 }, { uid: 1, file: { path: '../../text.txt' } }, function (err) { + assert.equal(err.message, '[[error:invalid-data]]'); + 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]]'); @@ -396,5 +410,33 @@ describe('Upload Controllers', function () { }); }); }); + + it('should upload regular file', function (done) { + helpers.uploadFile(nconf.get('url') + '/api/admin/upload/file', path.join(__dirname, '../test/files/test.png'), { + params: JSON.stringify({ + folder: 'system', + }), + }, jar, csrf_token, function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 200); + assert(Array.isArray(body)); + assert.equal(body[0].url, '/assets/uploads/system/test.png'); + assert(file.existsSync(path.join(nconf.get('upload_path'), 'system', 'test.png'))); + done(); + }); + }); + + it('should fail to upload regular file in wrong directory', function (done) { + helpers.uploadFile(nconf.get('url') + '/api/admin/upload/file', path.join(__dirname, '../test/files/test.png'), { + params: JSON.stringify({ + folder: '../../system', + }), + }, jar, csrf_token, function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 500); + assert.strictEqual(body.error, '[[error:invalid-path]]'); + done(); + }); + }); }); }); diff --git a/test/user.js b/test/user.js index c122d7f6b5..1e785c040a 100644 --- a/test/user.js +++ b/test/user.js @@ -936,31 +936,6 @@ describe('User', function () { }); }); - it('should upload profile picture', function (done) { - helpers.copyFile( - path.join(nconf.get('base_dir'), 'test/files/test.png'), - path.join(nconf.get('base_dir'), 'test/files/test_copy.png'), - function (err) { - assert.ifError(err); - var picture = { - path: path.join(nconf.get('base_dir'), 'test/files/test_copy.png'), - size: 7189, - name: 'test_copy.png', - type: 'image/png', - }; - User.uploadCroppedPicture({ - uid: uid, - file: picture, - }, function (err, uploadedPicture) { - assert.ifError(err); - assert.equal(uploadedPicture.url, '/assets/uploads/profile/' + uid + '-profileavatar.png'); - assert.equal(uploadedPicture.path, path.join(nconf.get('upload_path'), 'profile', uid + '-profileavatar.png')); - done(); - }); - } - ); - }); - it('should return error if profile image uploads disabled', function (done) { meta.config.allowProfileImageUploads = 0; var picture = { @@ -974,37 +949,15 @@ describe('User', function () { file: picture, }, function (err) { assert.equal(err.message, '[[error:profile-image-uploads-disabled]]'); - done(); - }); - }); - - it('should return error if profile image is too big', function (done) { - meta.config.allowProfileImageUploads = 1; - var picture = { - path: path.join(nconf.get('base_dir'), 'test/files/test_copy.png'), - size: 265000, - name: 'test.png', - type: 'image/png', - }; - - User.uploadCroppedPicture({ - uid: uid, - file: picture, - }, function (err) { - assert.equal(err.message, '[[error:file-too-big, 256]]'); + meta.config.allowProfileImageUploads = 1; done(); }); }); it('should return error if profile image has no mime type', function (done) { - var picture = { - path: path.join(nconf.get('base_dir'), 'test/files/test_copy.png'), - size: 7189, - name: 'test', - }; User.uploadCroppedPicture({ uid: uid, - file: picture, + imageData: 'data:image/invalid;base64,R0lGODlhPQBEAPeoAJosM/', }, function (err) { assert.equal(err.message, '[[error:invalid-image]]'); done();