🔒 Fixed magic link endpoint sending multiple emails

refs https://github.com/TryGhost/Team/issues/2024

Without validation it was possible to send a string of comma separated
email addresses to the endpoint, and an email would be sent to each
address, bypassing any rate limiting.

This bug does not allow for an authentication bypass exploit. It is purely a
spam email concern.

Credit: Sandip Maity <maitysandip925@gmail.com>

ghost/core/test/e2e-api/members/send-magic-link.test.js

@@ -27,6 +27,15 @@ describe('sendMagicLink', function () {
         mockManager.restore();
     });
 
+    it('Errors when passed multiple emails', async function () {
+        await membersAgent.post('/api/send-magic-link')
+            .body({
+                email: 'one@test.com,two@test.com',
+                emailType: 'signup'
+            })
+            .expectStatus(400);
+    });
+
     it('Creates a valid magic link with tokenData, and without urlHistory', async function () {
         const email = 'newly-created-user-magic-link-test@test.com';
         await membersAgent.post('/api/send-magic-link')

ghost/magic-link/lib/MagicLink.js

@@ -1,4 +1,9 @@
-const {IncorrectUsageError} = require('@tryghost/errors');
+const {IncorrectUsageError, BadRequestError} = require('@tryghost/errors');
+const {isEmail} = require('@tryghost/validator');
+const tpl = require('@tryghost/tpl');
+const messages = {
+    invalidEmail: 'Email is not valid'
+};
 
 /**
  * @typedef { import('nodemailer').Transporter } MailTransporter
@@ -52,6 +57,11 @@ class MagicLink {
      * @returns {Promise<{token: Token, info: SentMessageInfo}>}
      */
     async sendMagicLink(options) {
+        if (!isEmail(options.email)) {
+            throw new BadRequestError({
+                message: tpl(messages.invalidEmail)
+            });
+        }
         const token = await this.tokenProvider.create(options.tokenData);
 
         const type = options.type || 'signin';

ghost/magic-link/package.json

@@ -25,6 +25,8 @@
   },
   "dependencies": {
     "@tryghost/errors": "1.2.17",
+    "@tryghost/tpl": "0.1.18",
+    "@tryghost/validator": "0.1.29",
     "jsonwebtoken": "8.5.1"
   }
 }

ghost/magic-link/test/index.test.js

@@ -1,4 +1,4 @@
-const should = require('should');
+const assert = require('assert');
 const sinon = require('sinon');
 const MagicLink = require('../');
 const crypto = require('crypto');
@@ -8,10 +8,42 @@ const secret = crypto.randomBytes(64);
 
 describe('MagicLink', function () {
     it('Exports a function', function () {
-        should.equal(typeof MagicLink, 'function');
+        assert.equal(typeof MagicLink, 'function');
     });
 
     describe('#sendMagicLink', function () {
+        it('Throws when passed comma separated emails', async function () {
+            const options = {
+                tokenProvider: new MagicLink.JWTTokenProvider(secret),
+                getSigninURL: sandbox.stub().returns('FAKEURL'),
+                getText: sandbox.stub().returns('SOMETEXT'),
+                getHTML: sandbox.stub().returns('SOMEHTML'),
+                getSubject: sandbox.stub().returns('SOMESUBJECT'),
+                transporter: {
+                    sendMail: sandbox.stub().resolves()
+                }
+            };
+            const service = new MagicLink(options);
+
+            const args = {
+                email: 'one@email.com,two@email.com',
+                tokenData: {
+                    id: '420'
+                },
+                type: 'blazeit',
+                referrer: 'https://whatever.com'
+            };
+
+            let errored = false;
+            try {
+                await service.sendMagicLink(args);
+            } catch (err) {
+                errored = true;
+            } finally {
+                assert(errored, 'sendMagicLink should error when given comma separated emails');
+            }
+        });
+
         it('Sends an email to the user with a link generated from getSigninURL(token, type)', async function () {
             const options = {
                 tokenProvider: new MagicLink.JWTTokenProvider(secret),
@@ -35,23 +67,23 @@ describe('MagicLink', function () {
             };
             const {token} = await service.sendMagicLink(args);
 
-            should.ok(options.getSigninURL.calledOnce);
+            assert(options.getSigninURL.calledOnce);
-            should.ok(options.getSigninURL.firstCall.calledWithExactly(token, 'blazeit', 'https://whatever.com'));
+            assert(options.getSigninURL.firstCall.calledWithExactly(token, 'blazeit', 'https://whatever.com'));
 
-            should.ok(options.getText.calledOnce);
+            assert(options.getText.calledOnce);
-            should.ok(options.getText.firstCall.calledWithExactly('FAKEURL', 'blazeit', 'test@example.com'));
+            assert(options.getText.firstCall.calledWithExactly('FAKEURL', 'blazeit', 'test@example.com'));
 
-            should.ok(options.getHTML.calledOnce);
+            assert(options.getHTML.calledOnce);
-            should.ok(options.getHTML.firstCall.calledWithExactly('FAKEURL', 'blazeit', 'test@example.com'));
+            assert(options.getHTML.firstCall.calledWithExactly('FAKEURL', 'blazeit', 'test@example.com'));
 
-            should.ok(options.getSubject.calledOnce);
+            assert(options.getSubject.calledOnce);
-            should.ok(options.getSubject.firstCall.calledWithExactly('blazeit'));
+            assert(options.getSubject.firstCall.calledWithExactly('blazeit'));
 
-            should.ok(options.transporter.sendMail.calledOnce);
+            assert(options.transporter.sendMail.calledOnce);
-            should.equal(options.transporter.sendMail.firstCall.args[0].to, args.email);
+            assert.equal(options.transporter.sendMail.firstCall.args[0].to, args.email);
-            should.equal(options.transporter.sendMail.firstCall.args[0].subject, options.getSubject.firstCall.returnValue);
+            assert.equal(options.transporter.sendMail.firstCall.args[0].subject, options.getSubject.firstCall.returnValue);
-            should.equal(options.transporter.sendMail.firstCall.args[0].text, options.getText.firstCall.returnValue);
+            assert.equal(options.transporter.sendMail.firstCall.args[0].text, options.getText.firstCall.returnValue);
-            should.equal(options.transporter.sendMail.firstCall.args[0].html, options.getHTML.firstCall.returnValue);
+            assert.equal(options.transporter.sendMail.firstCall.args[0].html, options.getHTML.firstCall.returnValue);
         });
     });
 
@@ -78,7 +110,7 @@ describe('MagicLink', function () {
             const {token} = await service.sendMagicLink(args);
             const data = await service.getDataFromToken(token);
 
-            should.deepEqual(data.id, args.tokenData.id);
+            assert.deepEqual(data.id, args.tokenData.id);
         });
     });
 });