hyperzlib
/
nodebb
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: vulnerability in cover and admin uploads (#8419)

Browse Source 
* fix: vulnerability in cover and admin uploads

* fix: remove old test

* fix: update tests
v1.18.x
Barış Soner Uşaklı 5 years ago committed by GitHub
parent 76c577fa3c
commit 48b41debe6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 66 additions and 72 deletions
Show Stats Download Patch File Download Diff File

1
public/src/client/account/edit.js
Unescape Escape View File

@ -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,

3
src/file.js
Unescape Escape View File

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

2
src/image.js
Unescape Escape View File

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

29
src/user/picture.js
Unescape Escape View File

@ -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) {

8
test/file.js
Unescape Escape View File

@ -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) {

2
test/helpers/index.js
Unescape Escape View File

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

42
test/uploads.js
Unescape Escape View File

@ -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();
});
});
});
});

51
test/user.js
Unescape Escape View File

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

Write Preview
Loading…
Cancel
Save