mirror of
https://github.com/NodeBB/NodeBB.git
synced 2025-10-26 16:46:12 +01:00
feat: rate limit file uploads
This commit is contained in:
committed by
Andrew Rodrigues
parent
0a3a22db9d
commit
a9978fcfd2
@@ -36,6 +36,8 @@
|
|||||||
"allowAccountDelete": 1,
|
"allowAccountDelete": 1,
|
||||||
"privateUploads": 0,
|
"privateUploads": 0,
|
||||||
"allowedFileExtensions": "png,jpg,bmp,txt",
|
"allowedFileExtensions": "png,jpg,bmp,txt",
|
||||||
|
"uploadRateLimitThreshold": 10,
|
||||||
|
"uploadRateLimitCooldown": 60,
|
||||||
"allowUserHomePage": 1,
|
"allowUserHomePage": 1,
|
||||||
"allowMultipleBadges": 0,
|
"allowMultipleBadges": 0,
|
||||||
"maximumFileSize": 2048,
|
"maximumFileSize": 2048,
|
||||||
|
|||||||
@@ -21,6 +21,10 @@
|
|||||||
"topic-thumb-size": "Topic Thumb Size",
|
"topic-thumb-size": "Topic Thumb Size",
|
||||||
"allowed-file-extensions": "Allowed File Extensions",
|
"allowed-file-extensions": "Allowed File Extensions",
|
||||||
"allowed-file-extensions-help": "Enter comma-separated list of file extensions here (e.g. <code>pdf,xls,doc</code>). An empty list means all extensions are allowed.",
|
"allowed-file-extensions-help": "Enter comma-separated list of file extensions here (e.g. <code>pdf,xls,doc</code>). An empty list means all extensions are allowed.",
|
||||||
|
"upload-limit-threshold": "Rate limit user uploads to:",
|
||||||
|
"upload-limit-threshold-per": "per",
|
||||||
|
"upload-limit-threshold-per-minute": "Minute",
|
||||||
|
"upload-limit-threshold-per-minutes": "Minutes",
|
||||||
"profile-avatars": "Profile Avatars",
|
"profile-avatars": "Profile Avatars",
|
||||||
"allow-profile-image-uploads": "Allow users to upload profile images",
|
"allow-profile-image-uploads": "Allow users to upload profile images",
|
||||||
"convert-profile-image-png": "Convert profile image uploads to PNG",
|
"convert-profile-image-png": "Convert profile image uploads to PNG",
|
||||||
|
|||||||
@@ -103,6 +103,7 @@
|
|||||||
"file-too-big": "Maximum allowed file size is %1 kB - please upload a smaller file",
|
"file-too-big": "Maximum allowed file size is %1 kB - please upload a smaller file",
|
||||||
"guest-upload-disabled": "Guest uploading has been disabled",
|
"guest-upload-disabled": "Guest uploading has been disabled",
|
||||||
"cors-error": "Unable to upload image due to misconfigured CORS",
|
"cors-error": "Unable to upload image due to misconfigured CORS",
|
||||||
|
"upload-ratelimit-reached": "You have uploaded too many files at one time. Please try again later.",
|
||||||
|
|
||||||
"scheduling-to-past": "Please select a date in the future.",
|
"scheduling-to-past": "Please select a date in the future.",
|
||||||
"invalid-schedule-date": "Please enter a valid date and time.",
|
"invalid-schedule-date": "Please enter a valid date and time.",
|
||||||
|
|||||||
@@ -66,6 +66,7 @@ Object.assign(middleware, {
|
|||||||
require('./render')(middleware);
|
require('./render')(middleware);
|
||||||
require('./maintenance')(middleware);
|
require('./maintenance')(middleware);
|
||||||
require('./user')(middleware);
|
require('./user')(middleware);
|
||||||
|
require('./uploads')(middleware);
|
||||||
require('./headers')(middleware);
|
require('./headers')(middleware);
|
||||||
require('./expose')(middleware);
|
require('./expose')(middleware);
|
||||||
middleware.assert = require('./assert');
|
middleware.assert = require('./assert');
|
||||||
|
|||||||
32
src/middleware/uploads.js
Normal file
32
src/middleware/uploads.js
Normal file
@@ -0,0 +1,32 @@
|
|||||||
|
'use strict';
|
||||||
|
|
||||||
|
const LRU = require('lru-cache');
|
||||||
|
const meta = require('../meta');
|
||||||
|
const helpers = require('./helpers');
|
||||||
|
const user = require('../user');
|
||||||
|
const controllerHelpers = require('../controllers/helpers');
|
||||||
|
|
||||||
|
const cache = new LRU({
|
||||||
|
maxAge: meta.config.uploadRateLimitThreshold * 1000,
|
||||||
|
});
|
||||||
|
|
||||||
|
module.exports = function (middleware) {
|
||||||
|
middleware.ratelimitUploads = helpers.try(async (req, res, next) => {
|
||||||
|
const { uid } = req;
|
||||||
|
if (!uid) {
|
||||||
|
return controllerHelpers.notAllowed(req, res);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!meta.config.uploadRateLimitThreshold || await user.isAdminOrGlobalMod(req.uid)) {
|
||||||
|
return next();
|
||||||
|
}
|
||||||
|
|
||||||
|
const count = (cache.peek(`${uid}:uploaded_file_count`) || 0) + req.files.files.length;
|
||||||
|
if (count > meta.config.uploadRateLimitThreshold) {
|
||||||
|
return next(new Error(['[[error:upload-ratelimit-reached]]']));
|
||||||
|
}
|
||||||
|
|
||||||
|
cache.set(`${uid}:uploaded_file_count`, count);
|
||||||
|
next();
|
||||||
|
});
|
||||||
|
};
|
||||||
@@ -27,8 +27,14 @@ module.exports = function (app, middleware, controllers) {
|
|||||||
|
|
||||||
const multipart = require('connect-multiparty');
|
const multipart = require('connect-multiparty');
|
||||||
const multipartMiddleware = multipart();
|
const multipartMiddleware = multipart();
|
||||||
const middlewares = [middleware.maintenanceMode, multipartMiddleware, middleware.validateFiles, middleware.applyCSRF];
|
const middlewares = [
|
||||||
router.post('/post/upload', middlewares, uploadsController.uploadPost);
|
middleware.maintenanceMode,
|
||||||
|
multipartMiddleware,
|
||||||
|
middleware.validateFiles,
|
||||||
|
middleware.ratelimitUploads,
|
||||||
|
middleware.applyCSRF,
|
||||||
|
];
|
||||||
|
|
||||||
|
router.post('/post/upload', middlewares, uploadsController.uploadPost);
|
||||||
router.post('/user/:userslug/uploadpicture', middlewares.concat([middleware.exposeUid, middleware.authenticateRequest, middleware.ensureLoggedIn, middleware.canViewUsers, middleware.checkAccountPermissions]), controllers.accounts.edit.uploadPicture);
|
router.post('/user/:userslug/uploadpicture', middlewares.concat([middleware.exposeUid, middleware.authenticateRequest, middleware.ensureLoggedIn, middleware.canViewUsers, middleware.checkAccountPermissions]), controllers.accounts.edit.uploadPicture);
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ module.exports = function () {
|
|||||||
setupApiRoute(router, 'delete', '/:tid/tags', [...middlewares, middleware.assert.topic], controllers.write.topics.deleteTags);
|
setupApiRoute(router, 'delete', '/:tid/tags', [...middlewares, middleware.assert.topic], controllers.write.topics.deleteTags);
|
||||||
|
|
||||||
setupApiRoute(router, 'get', '/:tid/thumbs', [], controllers.write.topics.getThumbs);
|
setupApiRoute(router, 'get', '/:tid/thumbs', [], controllers.write.topics.getThumbs);
|
||||||
setupApiRoute(router, 'post', '/:tid/thumbs', [multipartMiddleware, middleware.validateFiles, ...middlewares], controllers.write.topics.addThumb);
|
setupApiRoute(router, 'post', '/:tid/thumbs', [multipartMiddleware, middleware.validateFiles, middleware.ratelimitUploads, ...middlewares], controllers.write.topics.addThumb);
|
||||||
setupApiRoute(router, 'put', '/:tid/thumbs', [...middlewares, middleware.checkRequired.bind(null, ['tid'])], controllers.write.topics.migrateThumbs);
|
setupApiRoute(router, 'put', '/:tid/thumbs', [...middlewares, middleware.checkRequired.bind(null, ['tid'])], controllers.write.topics.migrateThumbs);
|
||||||
setupApiRoute(router, 'delete', '/:tid/thumbs', [...middlewares, middleware.checkRequired.bind(null, ['path'])], controllers.write.topics.deleteThumb);
|
setupApiRoute(router, 'delete', '/:tid/thumbs', [...middlewares, middleware.checkRequired.bind(null, ['path'])], controllers.write.topics.deleteThumb);
|
||||||
setupApiRoute(router, 'put', '/:tid/thumbs/order', [...middlewares, middleware.checkRequired.bind(null, ['path', 'order'])], controllers.write.topics.reorderThumbs);
|
setupApiRoute(router, 'put', '/:tid/thumbs/order', [...middlewares, middleware.checkRequired.bind(null, ['path', 'order'])], controllers.write.topics.reorderThumbs);
|
||||||
|
|||||||
@@ -101,6 +101,26 @@
|
|||||||
[[admin/settings/uploads:allowed-file-extensions-help]]
|
[[admin/settings/uploads:allowed-file-extensions-help]]
|
||||||
</p>
|
</p>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
<div class="form-group">
|
||||||
|
<label for="uploadRateLimitThreshold">[[admin/settings/uploads:upload-limit-threshold]]</label>
|
||||||
|
<div class="row">
|
||||||
|
<div class="col-xs-2">
|
||||||
|
<input type="text" class="form-control" data-field="uploadRateLimitThreshold" />
|
||||||
|
</div>
|
||||||
|
<div class="col-xs-1">
|
||||||
|
<label style="vertical-align: -5px;">[[admin/settings/uploads:upload-limit-threshold-per]]</label>
|
||||||
|
</div>
|
||||||
|
<div class="col-xs-2">
|
||||||
|
<select class="form-control" data-field="uploadRateLimitCooldown">
|
||||||
|
<option value="60">1 [[admin/settings/uploads:upload-limit-threshold-per-minute]]</option>
|
||||||
|
<option value="300">5 [[admin/settings/uploads:upload-limit-threshold-per-minutes]]</option>
|
||||||
|
<option value="900">15 [[admin/settings/uploads:upload-limit-threshold-per-minutes]]</option>
|
||||||
|
<option value="3600">60 [[admin/settings/uploads:upload-limit-threshold-per-minutes]]</option>
|
||||||
|
</select>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
</form>
|
</form>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|||||||
@@ -24,6 +24,7 @@ describe('Upload Controllers', () => {
|
|||||||
let pid;
|
let pid;
|
||||||
let adminUid;
|
let adminUid;
|
||||||
let regularUid;
|
let regularUid;
|
||||||
|
let maliciousUid;
|
||||||
|
|
||||||
before((done) => {
|
before((done) => {
|
||||||
async.series({
|
async.series({
|
||||||
@@ -39,12 +40,16 @@ describe('Upload Controllers', () => {
|
|||||||
regularUid: function (next) {
|
regularUid: function (next) {
|
||||||
user.create({ username: 'regular', password: 'zugzug' }, next);
|
user.create({ username: 'regular', password: 'zugzug' }, next);
|
||||||
},
|
},
|
||||||
|
maliciousUid: function (next) {
|
||||||
|
user.create({ username: 'malicioususer', password: 'herpderp' }, next);
|
||||||
|
},
|
||||||
}, (err, results) => {
|
}, (err, results) => {
|
||||||
if (err) {
|
if (err) {
|
||||||
return done(err);
|
return done(err);
|
||||||
}
|
}
|
||||||
adminUid = results.adminUid;
|
adminUid = results.adminUid;
|
||||||
regularUid = results.regularUid;
|
regularUid = results.regularUid;
|
||||||
|
maliciousUid = results.maliciousUid;
|
||||||
cid = results.category.cid;
|
cid = results.category.cid;
|
||||||
|
|
||||||
topics.post({ uid: adminUid, title: 'test topic title', content: 'test topic content', cid: results.category.cid }, (err, result) => {
|
topics.post({ uid: adminUid, title: 'test topic title', content: 'test topic content', cid: results.category.cid }, (err, result) => {
|
||||||
@@ -132,7 +137,6 @@ describe('Upload Controllers', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
||||||
it('should upload a file to a post', (done) => {
|
it('should upload a file to a post', (done) => {
|
||||||
const oldValue = meta.config.allowedFileExtensions;
|
const oldValue = meta.config.allowedFileExtensions;
|
||||||
meta.config.allowedFileExtensions = 'png,jpg,bmp,html';
|
meta.config.allowedFileExtensions = 'png,jpg,bmp,html';
|
||||||
@@ -157,16 +161,6 @@ describe('Upload Controllers', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should fail to upload image to post if image is broken', (done) => {
|
|
||||||
helpers.uploadFile(`${nconf.get('url')}/api/post/upload`, path.join(__dirname, '../test/files/brokenimage.png'), {}, jar, csrf_token, (err, res, body) => {
|
|
||||||
assert.ifError(err);
|
|
||||||
assert.strictEqual(res.statusCode, 500);
|
|
||||||
assert(body && body.status && body.status.message);
|
|
||||||
assert(body.status.message.startsWith('Input file has corrupt header: pngload: end of stream'));
|
|
||||||
done();
|
|
||||||
});
|
|
||||||
});
|
|
||||||
|
|
||||||
it('should fail if file is not an image', (done) => {
|
it('should fail if file is not an image', (done) => {
|
||||||
image.isFileTypeAllowed(path.join(__dirname, '../test/files/notanimage.png'), (err) => {
|
image.isFileTypeAllowed(path.join(__dirname, '../test/files/notanimage.png'), (err) => {
|
||||||
assert.strictEqual(err.message, 'Input file contains unsupported image format');
|
assert.strictEqual(err.message, 'Input file contains unsupported image format');
|
||||||
@@ -322,6 +316,47 @@ describe('Upload Controllers', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('regular user uploads rate limits', () => {
|
||||||
|
let jar;
|
||||||
|
let csrf_token;
|
||||||
|
|
||||||
|
before((done) => {
|
||||||
|
helpers.loginUser('malicioususer', 'herpderp', (err, _jar, _csrf_token) => {
|
||||||
|
assert.ifError(err);
|
||||||
|
jar = _jar;
|
||||||
|
csrf_token = _csrf_token;
|
||||||
|
privileges.global.give(['groups:upload:post:file'], 'registered-users', done);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should fail if the user exceeds the upload rate limit threshold', (done) => {
|
||||||
|
const oldValue = meta.config.allowedFileExtensions;
|
||||||
|
meta.config.allowedFileExtensions = 'png,jpg,bmp,html';
|
||||||
|
|
||||||
|
// why / 2? see: helpers.uploadFile for a weird quirk where we actually upload 2 files per upload in our tests.
|
||||||
|
async.times(meta.config.uploadRateLimitThreshold / 2, (i, next) => {
|
||||||
|
helpers.uploadFile(`${nconf.get('url')}/api/post/upload`, path.join(__dirname, '../test/files/503.html'), {}, jar, csrf_token, (err, res, body) => {
|
||||||
|
if (i + 1 > meta.config.uploadRateLimitThreshold / 2) {
|
||||||
|
assert.strictEqual(res.statusCode, 500);
|
||||||
|
assert.strictEqual(body.error, '[[error:upload-ratelimit-reached]]');
|
||||||
|
} else {
|
||||||
|
assert.ifError(err);
|
||||||
|
assert.strictEqual(res.statusCode, 200);
|
||||||
|
assert(body && body.status && body.response && body.response.images);
|
||||||
|
assert(Array.isArray(body.response.images));
|
||||||
|
assert(body.response.images[0].url);
|
||||||
|
}
|
||||||
|
|
||||||
|
next(err);
|
||||||
|
});
|
||||||
|
}, (err) => {
|
||||||
|
meta.config.allowedFileExtensions = oldValue;
|
||||||
|
assert.ifError(err);
|
||||||
|
done();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('admin uploads', () => {
|
describe('admin uploads', () => {
|
||||||
let jar;
|
let jar;
|
||||||
let csrf_token;
|
let csrf_token;
|
||||||
|
|||||||
Reference in New Issue
Block a user