From ee870b9fed8a344242349fe36581f4894f190c06 Mon Sep 17 00:00:00 2001 From: NickOvt Date: Mon, 23 Sep 2024 12:38:02 +0300 Subject: [PATCH] fix(api&imap-mailboxes): Added mailbox subpath and whole path max length limits to API and IMAP ZMS-169 (#732) * Added submission api endpoint to api docs generation * mailboxes.js, check for max subpath length and max count on create and rename of a mailbox * add mailboxes tests * IMAP rename err code fix. IMAP mailbox create and rename tests added * IMAP tests, remove magic numbers * mailboxes tests, remove magic numbers * mailboxes tests fix broken tests * remove repetitions --- imap-core/lib/commands/rename.js | 2 +- imap-core/test/protocol-test.js | 113 +++++++++++++++++++++++++++++ lib/api/mailboxes.js | 7 +- lib/consts.js | 8 +- lib/mailbox-handler.js | 42 +++++++++++ lib/schemas.js | 22 +++++- test/api/mailboxes-test.js | 121 ++++++++++++++++++++++++++++++- 7 files changed, 309 insertions(+), 6 deletions(-) diff --git a/imap-core/lib/commands/rename.js b/imap-core/lib/commands/rename.js index f51b9978..3a385eb3 100644 --- a/imap-core/lib/commands/rename.js +++ b/imap-core/lib/commands/rename.js @@ -85,7 +85,7 @@ module.exports = { this._server.loggelf(logdata); return callback(null, { response: 'NO', - code: 'TEMPFAIL' + code: err.code || 'TEMPFAIL' }); } diff --git a/imap-core/test/protocol-test.js b/imap-core/test/protocol-test.js index aaba0330..7a351707 100644 --- a/imap-core/test/protocol-test.js +++ b/imap-core/test/protocol-test.js @@ -13,6 +13,8 @@ let chunks = require('./fixtures/chunks'); let expect = chai.expect; chai.config.includeStack = true; +const { MAX_SUB_MAILBOXES, MAX_MAILBOX_NAME_LENGTH } = require('../../lib/consts.js'); + describe('IMAP Protocol integration tests', function () { this.timeout(100000); // eslint-disable-line no-invalid-this let port = 9993; @@ -472,6 +474,66 @@ describe('IMAP Protocol integration tests', function () { } ); }); + + it(`cannot create a mailbox with subpath length bigger than ${MAX_MAILBOX_NAME_LENGTH} chars`, function (done) { + let cmds = [ + 'T1 LOGIN testuser pass', + `T2 CREATE parent/child/${'a'.repeat(MAX_MAILBOX_NAME_LENGTH + 1)}`, + 'T3 CREATE parent/child', + 'T4 CREATE testfolder', + 'T5 LIST "" "*"', + 'T6 LOGOUT' + ]; + + testClient( + { + commands: cmds, + secure: true, + port + }, + function (resp) { + resp = resp.toString(); + expect(/^T2 NO \[CANNOT\]/m.test(resp)).to.be.true; + expect(/^T3 OK/m.test(resp)).to.be.true; + expect(/^T4 OK/m.test(resp)).to.be.true; + expect(resp.indexOf('\r\n* LIST (\\HasNoChildren) "/" "testfolder"\r\n') >= 0).to.be.true; + expect(resp.indexOf('\r\n* LIST (\\Noselect \\HasChildren) "/" "parent"\r\n') >= 0).to.be.true; + expect(resp.indexOf('\r\n* LIST (\\HasNoChildren) "/" "parent/child"\r\n') >= 0).to.be.true; + done(); + } + ); + }); + + it(`cannot create a mailbox with more than ${MAX_SUB_MAILBOXES} subpaths`, function (done) { + let cmds = ['T1 LOGIN testuser pass', `T2 CREATE tobechanged`, 'T3 CREATE parent/child', 'T4 CREATE testfolder', 'T5 LIST "" "*"', 'T6 LOGOUT']; + + let path = ''; + + for (let i = 0; i < MAX_SUB_MAILBOXES + 1; i++) { + path += `subpath${i}/`; + } + path = path.substring(0, path.length - 1); + + cmds[1] = `T2 CREATE ${path}`; + + testClient( + { + commands: cmds, + secure: true, + port + }, + function (resp) { + resp = resp.toString(); + expect(/^T2 NO \[CANNOT\]/m.test(resp)).to.be.true; + expect(/^T3 OK/m.test(resp)).to.be.true; + expect(/^T4 OK/m.test(resp)).to.be.true; + expect(resp.indexOf('\r\n* LIST (\\HasNoChildren) "/" "testfolder"\r\n') >= 0).to.be.true; + expect(resp.indexOf('\r\n* LIST (\\Noselect \\HasChildren) "/" "parent"\r\n') >= 0).to.be.true; + expect(resp.indexOf('\r\n* LIST (\\HasNoChildren) "/" "parent/child"\r\n') >= 0).to.be.true; + done(); + } + ); + }); }); describe('RENAME', function () { @@ -502,6 +564,57 @@ describe('IMAP Protocol integration tests', function () { } ); }); + + it('cannot rename a mailbox to a mailbox path where subpath length is bigger than max allowed', function (done) { + let cmds = [ + 'T1 LOGIN testuser pass', + 'T2 CREATE testfolder', + `T3 RENAME testfolder parent/child/${'a'.repeat(MAX_MAILBOX_NAME_LENGTH + 1)}`, + 'T5 LIST "" "*"', + 'T6 LOGOUT' + ]; + + testClient( + { + commands: cmds, + secure: true, + port + }, + function (resp) { + resp = resp.toString(); + expect(/^T3 NO \[CANNOT\]/m.test(resp)).to.be.true; + expect(resp.indexOf('\r\n* LIST (\\HasNoChildren) "/" "testfolder"\r\n') >= 0).to.be.true; + done(); + } + ); + }); + + it('cannot rename a mailbox to a mailbox path where there are more than max subpath count', function (done) { + let cmds = ['T1 LOGIN testuser pass', 'T2 CREATE testfolder', `T3 RENAME testfolder parent/child`, 'T5 LIST "" "*"', 'T6 LOGOUT']; + + let path = ''; + + for (let i = 0; i < MAX_SUB_MAILBOXES + 1; i++) { + path += `subpath${i}/`; + } + path = path.substring(0, path.length - 1); + + cmds[2] = `T3 RENAME testfolder ${path}`; + + testClient( + { + commands: cmds, + secure: true, + port + }, + function (resp) { + resp = resp.toString(); + expect(/^T3 NO \[CANNOT\]/m.test(resp)).to.be.true; + expect(resp.indexOf('\r\n* LIST (\\HasNoChildren) "/" "testfolder"\r\n') >= 0).to.be.true; + done(); + } + ); + }); }); describe('DELETE', function () { diff --git a/lib/api/mailboxes.js b/lib/api/mailboxes.js index 7593614a..327c37c5 100644 --- a/lib/api/mailboxes.js +++ b/lib/api/mailboxes.js @@ -6,10 +6,11 @@ const imapTools = require('../../imap-core/lib/imap-tools'); const tools = require('../tools'); const roles = require('../roles'); const util = require('util'); -const { sessSchema, sessIPSchema, booleanSchema } = require('../schemas'); +const { sessSchema, sessIPSchema, booleanSchema, mailboxPathValidator } = require('../schemas'); const { userId, mailboxId } = require('../schemas/request/general-schemas'); const { successRes } = require('../schemas/response/general-schemas'); const { GetMailboxesResult } = require('../schemas/response/mailboxes-schemas'); +const { MAX_MAILBOX_NAME_LENGTH, MAX_SUB_MAILBOXES } = require('../consts'); module.exports = (db, server, mailboxHandler) => { const getMailboxCounter = util.promisify(tools.getMailboxCounter); @@ -281,6 +282,8 @@ module.exports = (db, server, mailboxHandler) => { requestBody: { path: Joi.string() .regex(/\/{2,}|\/$/, { invert: true }) + .max(MAX_MAILBOX_NAME_LENGTH * MAX_SUB_MAILBOXES + 127) + .custom(mailboxPathValidator, 'Mailbox path validation') .required() .description('Full path of the mailbox, folders are separated by slashes, ends with the mailbox name (unicode string)'), hidden: booleanSchema.default(false).description('Is the folder hidden or not. Hidden folders can not be opened in IMAP.'), @@ -539,6 +542,8 @@ module.exports = (db, server, mailboxHandler) => { requestBody: { path: Joi.string() .regex(/\/{2,}|\/$/, { invert: true }) + .max(MAX_MAILBOX_NAME_LENGTH * MAX_SUB_MAILBOXES + 127) + .custom(mailboxPathValidator, 'Mailbox path validation') .description('Full path of the mailbox, use this to rename an existing Mailbox'), retention: Joi.number() .empty('') diff --git a/lib/consts.js b/lib/consts.js index 12806460..0d180f9a 100644 --- a/lib/consts.js +++ b/lib/consts.js @@ -137,5 +137,11 @@ module.exports = { MAX_FILTERS: 400, // maximum amount of mailboxes per user - MAX_MAILBOXES: 1500 + MAX_MAILBOXES: 1500, + + // Max length of a mailbox subpath element + MAX_MAILBOX_NAME_LENGTH: 512, + + // Number of mailbox subpaths in a single mailbox path + MAX_SUB_MAILBOXES: 128 }; diff --git a/lib/mailbox-handler.js b/lib/mailbox-handler.js index aa8be17d..5658fb28 100644 --- a/lib/mailbox-handler.js +++ b/lib/mailbox-handler.js @@ -4,6 +4,7 @@ const ObjectId = require('mongodb').ObjectId; const ImapNotifier = require('./imap-notifier'); const { publish, MAILBOX_CREATED, MAILBOX_RENAMED, MAILBOX_DELETED } = require('./events'); const { SettingsHandler } = require('./settings-handler'); +const { MAX_MAILBOX_NAME_LENGTH, MAX_SUB_MAILBOXES } = require('./consts'); class MailboxHandler { constructor(options) { @@ -40,6 +41,26 @@ class MailboxHandler { throw err; } + const splittedPathArr = path.split('/'); + + if (splittedPathArr.length > MAX_SUB_MAILBOXES) { + // too many subpaths + const err = new Error(`The mailbox path cannot be more than ${MAX_SUB_MAILBOXES} levels deep`); + err.code = 'CANNOT'; + err.responseCode = 400; + throw err; + } + + for (const pathPart of splittedPathArr) { + if (pathPart.length > MAX_MAILBOX_NAME_LENGTH) { + // individual path part longer than specified max + const err = new Error(`Any part of the mailbox path cannot be longer than ${MAX_MAILBOX_NAME_LENGTH} chars`); + err.code = 'CANNOT'; + err.responseCode = 400; + throw err; + } + } + let mailboxData = await this.database.collection('mailboxes').findOne({ user, path }); if (mailboxData) { @@ -118,6 +139,27 @@ class MailboxHandler { if (err) { return callback(err); } + + const splittedPathArr = newname.split('/'); + + if (splittedPathArr.length > MAX_SUB_MAILBOXES) { + // too many subpaths + const err = new Error(`The mailbox path cannot be more than ${MAX_SUB_MAILBOXES} levels deep`); + err.code = 'CANNOT'; + err.responseCode = 400; + return callback(err, 'CANNOT'); + } + + for (const pathPart of splittedPathArr) { + if (pathPart.length > MAX_MAILBOX_NAME_LENGTH) { + // individual path part longer than specified max + const err = new Error(`Any part of the mailbox path cannot be longer than ${MAX_MAILBOX_NAME_LENGTH} chars`); + err.code = 'CANNOT'; + err.responseCode = 400; + return callback(err, 'CANNOT'); + } + } + if (!mailboxData) { const err = new Error('Mailbox update failed with code NoSuchMailbox'); err.code = 'NONEXISTENT'; diff --git a/lib/schemas.js b/lib/schemas.js index 9da57058..41c0e80a 100644 --- a/lib/schemas.js +++ b/lib/schemas.js @@ -2,6 +2,7 @@ const EJSON = require('mongodb-extended-json'); const Joi = require('joi'); +const { MAX_SUB_MAILBOXES, MAX_MAILBOX_NAME_LENGTH } = require('./consts'); const sessSchema = Joi.string().max(255).label('Session identifier').description('Session identifier for the logs'); const sessIPSchema = Joi.string() @@ -82,6 +83,24 @@ const metaDataValidator = () => (value, helpers) => { return strValue; }; +const mailboxPathValidator = (value, helpers) => { + const splittedPathArr = value.split('/'); + + if (splittedPathArr.length > MAX_SUB_MAILBOXES) { + // too many paths + return helpers.message(`The mailbox path cannot be more than ${MAX_SUB_MAILBOXES} levels deep`); + } + + for (const pathPart of splittedPathArr) { + if (pathPart.length > MAX_MAILBOX_NAME_LENGTH) { + // part too long error + return helpers.message(`Any part of the mailbox path cannot be longer than ${MAX_MAILBOX_NAME_LENGTH} chars long`); + } + } + + return value; +}; + const mongoCursorSchema = Joi.string().trim().empty('').custom(mongoCursorValidator({}), 'Cursor validation').max(1024); const pageLimitSchema = Joi.number().default(20).min(1).max(250).label('Page size'); const pageNrSchema = Joi.number().default(1).label('Page number').description('Current page number. Informational only, page numbers start from 1'); @@ -111,5 +130,6 @@ module.exports = { pageLimitSchema, booleanSchema, metaDataSchema, - usernameSchema + usernameSchema, + mailboxPathValidator }; diff --git a/test/api/mailboxes-test.js b/test/api/mailboxes-test.js index 87da78c1..3dea99b6 100644 --- a/test/api/mailboxes-test.js +++ b/test/api/mailboxes-test.js @@ -11,12 +11,15 @@ const expect = chai.expect; chai.config.includeStack = true; const config = require('wild-config'); +const { MAX_MAILBOX_NAME_LENGTH, MAX_SUB_MAILBOXES } = require('../../lib/consts'); + const server = supertest.agent(`http://127.0.0.1:${config.api.port}`); describe('Mailboxes tests', function () { this.timeout(10000); // eslint-disable-line no-invalid-this let user; + let mailboxForPut; before(async () => { // ensure that we have an existing user account @@ -33,6 +36,9 @@ describe('Mailboxes tests', function () { expect(response.body.id).to.exist; user = response.body.id; + + const responseMailboxPutCreate = await server.post(`/users/${user}/mailboxes`).send({ path: '/path/for/put', hidden: false, retention: 0 }).expect(200); + mailboxForPut = responseMailboxPutCreate.body.id; }); after(async () => { @@ -142,7 +148,7 @@ describe('Mailboxes tests', function () { expect(response.body.success).to.be.true; expect(response.body.results).to.not.be.empty; - expect(response.body.results.length).to.be.equal(8); + expect(response.body.results.length).to.be.equal(9); }); it('should GET /users/{user}/mailboxes expect success / all params', async () => { @@ -158,7 +164,7 @@ describe('Mailboxes tests', function () { expect(response.body.success).to.be.true; expect(response.body.results).to.not.be.empty; - expect(response.body.results.length).to.be.equal(8); + expect(response.body.results.length).to.be.equal(9); }); it('should GET /users/{user}/mailboxes expect failure / params incorrect type', async () => { @@ -434,4 +440,115 @@ describe('Mailboxes tests', function () { expect(response.body.error).to.be.equal('Mailbox deletion failed with code DisallowedMailboxMethod'); expect(response.body.code).to.be.equal('DisallowedMailboxMethod'); }); + + it('should POST /users/{user}/mailboxes expect failure / too many subpaths in mailbox path', async () => { + let path = ''; + + for (let i = 0; i < MAX_SUB_MAILBOXES + 1; i++) { + path += `subpath${i}/`; + } + + path = path.substring(0, path.length - 1); + const response = await server.post(`/users/${user}/mailboxes`).send({ path, hidden: false }).expect(400); + + expect(response.body.code).to.eq('InputValidationError'); + expect(response.body.error).to.eq(`The mailbox path cannot be more than ${MAX_SUB_MAILBOXES} levels deep`); + }); + + it('should POST /users/{user}/mailboxes expect failure / subpath too long', async () => { + let path = ''; + + for (let i = 0; i < 16; i++) { + if (i % 5 === 0) { + // every fifth + path += `${'a'.repeat(MAX_MAILBOX_NAME_LENGTH + 1)}/`; + } else { + path += `subpath${i}/`; + } + } + + path = path.substring(0, path.length - 1); + const response = await server.post(`/users/${user}/mailboxes`).send({ path, hidden: false }).expect(400); + + expect(response.body.code).to.eq('InputValidationError'); + expect(response.body.error).to.eq(`Any part of the mailbox path cannot be longer than ${MAX_MAILBOX_NAME_LENGTH} chars long`); + }); + + it('should POST /users/{user}/mailboxes expect success / edge case for subpath length and subpath count', async () => { + let path = ''; + + for (let i = 0; i < MAX_SUB_MAILBOXES; i++) { + path += `${'a'.repeat(MAX_MAILBOX_NAME_LENGTH)}/`; + } + + path = path.substring(0, path.length - 1); + const response = await server.post(`/users/${user}/mailboxes`).send({ path, hidden: false }).expect(200); + + expect(response.body.success).to.be.true; + expect(response.body.id).to.not.be.empty; + }); + + it('should PUT /users/{user}/mailboxes/{mailbox} expect failure / too many subpaths in mailbox path', async () => { + let path = ''; + + for (let i = 0; i < MAX_SUB_MAILBOXES + 1; i++) { + path += `subpath${i}/`; + } + + path = path.substring(0, path.length - 1); + const response = await server.put(`/users/${user}/mailboxes/${mailboxForPut}`).send({ path, hidden: false }).expect(400); + + expect(response.body.code).to.eq('InputValidationError'); + expect(response.body.error).to.eq(`The mailbox path cannot be more than ${MAX_SUB_MAILBOXES} levels deep`); + }); + + it('should PUT /users/{user}/mailboxes/{mailbox} expect failure / subpath too long', async () => { + let path = ''; + + for (let i = 0; i < 16; i++) { + if (i % 5 === 0) { + // every fifth + path += `${'a'.repeat(MAX_MAILBOX_NAME_LENGTH + 1)}/`; + } else { + path += `subpath${i}/`; + } + } + + path = path.substring(0, path.length - 1); + const response = await server.put(`/users/${user}/mailboxes/${mailboxForPut}`).send({ path, hidden: false }).expect(400); + + expect(response.body.code).to.eq('InputValidationError'); + expect(response.body.error).to.eq(`Any part of the mailbox path cannot be longer than ${MAX_MAILBOX_NAME_LENGTH} chars long`); + }); + + it('should PUT /users/{user}/mailboxes/{mailbox} expect success / edge case for subpath length and subpath count', async () => { + let path = ''; + + for (let i = 0; i < MAX_SUB_MAILBOXES; i++) { + path += `${`${i % 10}`.repeat(MAX_MAILBOX_NAME_LENGTH)}/`; + } + + path = path.substring(0, path.length - 1); + const response = await server.put(`/users/${user}/mailboxes/${mailboxForPut}`).send({ path, hidden: false }).expect(200); + + expect(response.body.success).to.be.true; + }); + + it('should POST /users/{user}/mailboxes expect failure / trailing slash', async () => { + let path = 'somepath/abc/'; + + const response = await server.post(`/users/${user}/mailboxes`).send({ path, hidden: false }).expect(400); + + expect(response.body.code).to.eq('InputValidationError'); + expect(response.body.error).to.eq('"path" with value "somepath/abc/" matches the inverted pattern: /\\/{2,}|\\/$/'); + }); + + it('should PUT /users/{user}/mailboxes/{mailbox} expect failure / trailing slash', async () => { + let path = 'somepath/abc/'; + + const response = await server.put(`/users/${user}/mailboxes/${mailboxForPut}`).send({ path, hidden: false }).expect(400); + + expect(response.body.code).to.eq('InputValidationError'); + expect(response.body.error).to.eq('"path" with value "somepath/abc/" matches the inverted pattern: /\\/{2,}|\\/$/'); + }); });