From 0029c444ad51dda4781295a64d19c09c19a8a3a6 Mon Sep 17 00:00:00 2001 From: Ronald Langeveld Date: Thu, 27 Jul 2023 08:46:50 +0200 Subject: [PATCH] Added test email rate limiting (#17505) refs https://github.com/TryGhost/Product/issues/3651 - This is a security fix that addresses an issue causing malicious users to abuse the test / preview email API endpoint. - We have multiple procedures in place now to limit such users. - First, we now only allow one email address to be passed into the `sendTestEmail` method. This method only have one purpose, which is to compliment the test email functionality within the Editor in Admin and therefore have no reason to send to more than one email address at a time. - We then add an additional rate limiter to prevent a user from making multiple requests, eg via a script. - The new imposed limit is 10 test emails per hour. --- .../server/web/api/endpoints/admin/routes.js | 3 +- .../shared/middleware/api/spam-prevention.js | 32 +++++++++++- .../server/web/shared/middleware/brute.js | 13 +++++ ghost/core/core/shared/config/defaults.json | 6 +++ .../config/env/config.testing-browser.json | 6 +++ .../config/env/config.testing-mysql.json | 7 ++- .../shared/config/env/config.testing.json | 6 +++ .../admin/email-preview-rate-limiter.test.js | 51 +++++++++++++++++++ ghost/email-service/lib/EmailController.js | 10 +++- .../test/email-controller.test.js | 28 ++++++++++ 10 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 ghost/core/test/e2e-api/admin/email-preview-rate-limiter.test.js diff --git a/ghost/core/core/server/web/api/endpoints/admin/routes.js b/ghost/core/core/server/web/api/endpoints/admin/routes.js index ecf0513ea9..a53f3d5109 100644 --- a/ghost/core/core/server/web/api/endpoints/admin/routes.js +++ b/ghost/core/core/server/web/api/endpoints/admin/routes.js @@ -314,7 +314,8 @@ module.exports = function apiRoutes() { // ## Email Preview router.get('/email_previews/posts/:id', mw.authAdminApi, http(api.email_previews.read)); - router.post('/email_previews/posts/:id', mw.authAdminApi, http(api.email_previews.sendTestEmail)); + // preview sending have an additional rate limiter to prevent abuse + router.post('/email_previews/posts/:id', shared.middleware.brute.previewEmailLimiter, mw.authAdminApi, http(api.email_previews.sendTestEmail)); // ## Emails router.get('/emails', mw.authAdminApi, http(api.emails.browse)); diff --git a/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js b/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js index d47490538b..90248b730a 100644 --- a/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js +++ b/ghost/core/core/server/web/shared/middleware/api/spam-prevention.js @@ -21,7 +21,8 @@ const messages = { context: 'Too many login attempts.' }, tooManyAttempts: 'Too many attempts.', - webmentionsBlock: 'Too many mention attempts' + webmentionsBlock: 'Too many mention attempts', + emailPreviewBlock: 'Only 10 test emails can be sent per hour' }; let spamPrivateBlock = spam.private_block || {}; let spamGlobalBlock = spam.global_block || {}; @@ -31,6 +32,7 @@ let spamUserLogin = spam.user_login || {}; let spamMemberLogin = spam.member_login || {}; let spamContentApiKey = spam.content_api_key || {}; let spamWebmentionsBlock = spam.webmentions_block || {}; +let spamEmailPreviewBlock = spam.email_preview_block || {}; let store; let memoryStore; @@ -43,6 +45,7 @@ let membersAuthInstance; let membersAuthEnumerationInstance; let userResetInstance; let contentApiKeyInstance; +let emailPreviewBlockInstance; const spamConfigKeys = ['freeRetries', 'minWait', 'maxWait', 'lifetime']; @@ -152,6 +155,32 @@ const webmentionsBlock = () => { return webmentionsBlockInstance; }; +const emailPreviewBlock = () => { + const ExpressBrute = require('express-brute'); + const BruteKnex = require('brute-knex'); + const db = require('../../../../data/db'); + + store = store || new BruteKnex({ + tablename: 'brute', + createTable: false, + knex: db.knex + }); + + emailPreviewBlockInstance = emailPreviewBlockInstance || new ExpressBrute(store, + extend({ + attachResetToRequest: false, + failCallback(req, res, next) { + return next(new errors.TooManyRequestsError({ + message: messages.emailPreviewBlock + })); + }, + handleStoreError: handleStoreError + }, pick(spamEmailPreviewBlock, spamConfigKeys)) + ); + + return emailPreviewBlockInstance; +}; + const membersAuth = () => { const ExpressBrute = require('express-brute'); const BruteKnex = require('brute-knex'); @@ -349,6 +378,7 @@ module.exports = { privateBlog: privateBlog, contentApiKey: contentApiKey, webmentionsBlock: webmentionsBlock, + emailPreviewBlock: emailPreviewBlock, reset: () => { store = undefined; memoryStore = undefined; diff --git a/ghost/core/core/server/web/shared/middleware/brute.js b/ghost/core/core/server/web/shared/middleware/brute.js index 496132888c..f64ad6f8ed 100644 --- a/ghost/core/core/server/web/shared/middleware/brute.js +++ b/ghost/core/core/server/web/shared/middleware/brute.js @@ -117,5 +117,18 @@ module.exports = { return _next('webmention_blocked'); } })(req, res, next); + }, + + /** + * Blocks preview email spam + */ + + previewEmailLimiter(req, res, next) { + return spamPrevention.emailPreviewBlock().getMiddleware({ + ignoreIP: false, + key(_req, _res, _next) { + return _next('preview_email_blocked'); + } + })(req, res, next); } }; diff --git a/ghost/core/core/shared/config/defaults.json b/ghost/core/core/shared/config/defaults.json index 191ddc4f78..853120745a 100644 --- a/ghost/core/core/shared/config/defaults.json +++ b/ghost/core/core/shared/config/defaults.json @@ -108,6 +108,12 @@ "maxWait": 100, "lifetime": 1000, "freeRetries": 100 + }, + "email_preview_block": { + "minWait": 360000, + "maxWait": 360000, + "lifetime": 3600, + "freeRetries": 10 } }, "caching": { diff --git a/ghost/core/core/shared/config/env/config.testing-browser.json b/ghost/core/core/shared/config/env/config.testing-browser.json index e534e72d0d..ee2ed00386 100644 --- a/ghost/core/core/shared/config/env/config.testing-browser.json +++ b/ghost/core/core/shared/config/env/config.testing-browser.json @@ -49,6 +49,12 @@ "maxWait": 100000, "lifetime": 3600, "freeRetries": 3 + }, + "email_preview_block": { + "minWait": 360000, + "maxWait": 360000, + "lifetime": 3600, + "freeRetries": 10 } }, "privacy": { diff --git a/ghost/core/core/shared/config/env/config.testing-mysql.json b/ghost/core/core/shared/config/env/config.testing-mysql.json index 0f4ea5acf2..3c15227b1c 100644 --- a/ghost/core/core/shared/config/env/config.testing-mysql.json +++ b/ghost/core/core/shared/config/env/config.testing-mysql.json @@ -50,8 +50,13 @@ "maxWait": 100000, "lifetime": 3600, "freeRetries": 3 + }, + "email_preview_block": { + "minWait": 360000, + "maxWait": 360000, + "lifetime": 3600, + "freeRetries": 10 } - }, "privacy": { "useTinfoil": true, diff --git a/ghost/core/core/shared/config/env/config.testing.json b/ghost/core/core/shared/config/env/config.testing.json index 84fcbd6b58..74e2f1ec78 100644 --- a/ghost/core/core/shared/config/env/config.testing.json +++ b/ghost/core/core/shared/config/env/config.testing.json @@ -49,6 +49,12 @@ "maxWait": 100000, "lifetime": 3600, "freeRetries": 3 + }, + "email_preview_block": { + "minWait": 360000, + "maxWait": 360000, + "lifetime": 3600, + "freeRetries": 10 } }, "privacy": { diff --git a/ghost/core/test/e2e-api/admin/email-preview-rate-limiter.test.js b/ghost/core/test/e2e-api/admin/email-preview-rate-limiter.test.js new file mode 100644 index 0000000000..8893b4bf45 --- /dev/null +++ b/ghost/core/test/e2e-api/admin/email-preview-rate-limiter.test.js @@ -0,0 +1,51 @@ +// Decided to have this test separately from the other email preview tests since the rate limiter would interfere with the other tests + +const {agentProvider, fixtureManager, mockManager, configUtils} = require('../../utils/e2e-framework'); +const sinon = require('sinon'); +const DomainEvents = require('@tryghost/domain-events'); + +async function allSettled() { + await DomainEvents.allSettled(); +} + +describe('Rate limiter', function () { + let agent; + + afterEach(function () { + mockManager.restore(); + sinon.restore(); + }); + + beforeEach(function () { + mockManager.mockMailgun(); + }); + + before(async function () { + agent = await agentProvider.getAdminAPIAgent(); + await fixtureManager.init('users', 'newsletters', 'posts'); + await agent.loginAsOwner(); + }); + + it('is rate limited against spammmer requests', async function () { + const testEmailSpamBlock = configUtils.config.get('spam').email_preview_block; + const requests = []; + for (let i = 0; i < testEmailSpamBlock.freeRetries + 1; i += 1) { + const req = await agent + .post(`email_previews/posts/${fixtureManager.get('posts', 0).id}/`) + .body({ + emails: ['test@ghost.org'] + }); + requests.push(req); + } + await Promise.all(requests); + + await agent + .post(`email_previews/posts/${fixtureManager.get('posts', 0).id}/`) + .body({ + emails: ['test@ghost.org'] + }) + .expectStatus(429); + + await allSettled(); + }); +}); diff --git a/ghost/email-service/lib/EmailController.js b/ghost/email-service/lib/EmailController.js index 2b85bc8b1b..6a17be0006 100644 --- a/ghost/email-service/lib/EmailController.js +++ b/ghost/email-service/lib/EmailController.js @@ -4,7 +4,8 @@ const tpl = require('@tryghost/tpl'); const messages = { postNotFound: 'Post not found.', noEmailsProvided: 'No emails provided.', - emailNotFound: 'Email not found.' + emailNotFound: 'Email not found.', + tooManyEmailsProvided: 'Too many emails provided. Maximum of 1 test email can be sent at once.' }; class EmailController { @@ -67,6 +68,13 @@ class EmailController { }); } + // test emails are limited to 1 + if (emails.length > 1) { + throw new errors.ValidationError({ + message: tpl(messages.tooManyEmailsProvided) + }); + } + await this.service.sendTestEmail(post, newsletter, segment, emails); } diff --git a/ghost/email-service/test/email-controller.test.js b/ghost/email-service/test/email-controller.test.js index 3538c7fda2..b0d2977103 100644 --- a/ghost/email-service/test/email-controller.test.js +++ b/ghost/email-service/test/email-controller.test.js @@ -230,6 +230,34 @@ describe('Email Controller', function () { }); assert.equal(result, undefined); }); + + it('throw if more than one email is provided', async function () { + const service = { + sendTestEmail: () => { + return Promise.resolve({id: 'mail@id'}); + } + }; + + const controller = new EmailController(service, { + models: { + Post: createModelClass({ + findOne: { + title: 'Post title' + } + }), + Newsletter: createModelClass() + } + }); + + await assert.rejects(controller.sendTestEmail({ + options: {}, + data: { + id: '123', + newsletter: 'newsletter-slug', + emails: ['example@example.com', 'example2@example.com'] + } + }), /Too many emails provided. Maximum of 1 test email can be sent at once./); + }); }); describe('retryFailedEmail', function () {