From 61da8c29ac1a407e65bf5702c4dd4f212aa443ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Tue, 7 Jan 2020 15:40:54 -0500 Subject: [PATCH] fix: group create/join/update name validation --- src/groups/create.js | 10 ++-- src/groups/update.js | 9 +++- src/socket.io/groups.js | 24 ++++++++- test/groups.js | 105 +++++++++++++++++++++++++++++++++++++--- 4 files changed, 136 insertions(+), 12 deletions(-) diff --git a/src/groups/create.js b/src/groups/create.js index 9a1f9c3954..76aec0afe0 100644 --- a/src/groups/create.js +++ b/src/groups/create.js @@ -16,7 +16,7 @@ module.exports = function (Groups) { const disableLeave = parseInt(data.disableLeave, 10) === 1 ? 1 : 0; const isHidden = parseInt(data.hidden, 10) === 1; - validateGroupName(data.name); + Groups.validateGroupName(data.name); const exists = await meta.userOrGroupExists(data.name); if (exists) { @@ -72,11 +72,15 @@ module.exports = function (Groups) { Groups.isPrivilegeGroup(data.name); } - function validateGroupName(name) { + Groups.validateGroupName = function (name) { if (!name) { throw new Error('[[error:group-name-too-short]]'); } + if (typeof name !== 'string') { + throw new Error('[[error:invalid-group-name]]'); + } + if (!Groups.isPrivilegeGroup(name) && name.length > meta.config.maximumGroupNameLength) { throw new Error('[[error:group-name-too-long]]'); } @@ -88,5 +92,5 @@ module.exports = function (Groups) { if (name.includes('/') || !utils.slugify(name)) { throw new Error('[[error:invalid-group-name]]'); } - } + }; }; diff --git a/src/groups/update.js b/src/groups/update.js index cc30cb8775..c7c0996557 100644 --- a/src/groups/update.js +++ b/src/groups/update.js @@ -54,7 +54,10 @@ module.exports = function (Groups) { payload.disableLeave = values.disableLeave ? '1' : '0'; } - await checkNameChange(groupName, values.name); + if (values.hasOwnProperty('name')) { + await checkNameChange(groupName, values.name); + } + if (values.hasOwnProperty('private')) { await updatePrivacy(groupName, values.private); } @@ -125,6 +128,10 @@ module.exports = function (Groups) { } async function checkNameChange(currentName, newName) { + Groups.validateGroupName(newName); + if (Groups.isPrivilegeGroup(newName)) { + throw new Error('[[error:invalid-group-name]]'); + } const currentSlug = utils.slugify(currentName); const newSlug = utils.slugify(newName); if (currentName === newName || currentSlug === newSlug) { diff --git a/src/socket.io/groups.js b/src/socket.io/groups.js index b00af300a6..e468125b27 100644 --- a/src/socket.io/groups.js +++ b/src/socket.io/groups.js @@ -22,6 +22,10 @@ SocketGroups.join = async (socket, data) => { throw new Error('[[error:invalid-uid]]'); } + if (typeof data.groupName !== 'string') { + throw new Error('[[error:invalid-group-name]]'); + } + if (data.groupName === 'administrators' || groups.isPrivilegeGroup(data.groupName)) { throw new Error('[[error:not-allowed]]'); } @@ -66,6 +70,10 @@ SocketGroups.leave = async (socket, data) => { throw new Error('[[error:invalid-uid]]'); } + if (typeof data.groupName !== 'string') { + throw new Error('[[error:invalid-group-name]]'); + } + if (data.groupName === 'administrators') { throw new Error('[[error:cant-remove-self-as-admin]]'); } @@ -104,6 +112,9 @@ SocketGroups.addMember = async (socket, data) => { }; async function isOwner(socket, data) { + if (typeof data.groupName !== 'string') { + throw new Error('[[error:invalid-group-name]]'); + } const results = await utils.promiseParallel({ isAdmin: await user.isAdministrator(socket.uid), isGlobalModerator: await user.isGlobalModerator(socket.uid), @@ -118,6 +129,9 @@ async function isOwner(socket, data) { } async function isInvited(socket, data) { + if (typeof data.groupName !== 'string') { + throw new Error('[[error:invalid-group-name]]'); + } const invited = await groups.isInvited(socket.uid, data.groupName); if (!invited) { throw new Error('[[error:not-invited]]'); @@ -171,6 +185,9 @@ SocketGroups.rejectAll = async (socket, data) => { }; async function acceptRejectAll(method, socket, data) { + if (typeof data.groupName !== 'string') { + throw new Error('[[error:invalid-group-name]]'); + } const uids = await groups.getPending(data.groupName); await Promise.all(uids.map(async (uid) => { await method(socket, { groupName: data.groupName, toUid: uid }); @@ -251,7 +268,7 @@ SocketGroups.kick = async (socket, data) => { SocketGroups.create = async (socket, data) => { if (!socket.uid) { throw new Error('[[error:no-privileges]]'); - } else if (groups.isPrivilegeGroup(data.name)) { + } else if (typeof data.name !== 'string' || groups.isPrivilegeGroup(data.name)) { throw new Error('[[error:invalid-group-name]]'); } @@ -260,6 +277,7 @@ SocketGroups.create = async (socket, data) => { throw new Error('[[error:no-privileges]]'); } data.ownerUid = socket.uid; + data.system = false; const groupData = await groups.create(data); logGroupEvent(socket, 'group-create', { groupName: data.name, @@ -338,7 +356,6 @@ SocketGroups.cover.update = async (socket, data) => { if (!socket.uid) { throw new Error('[[error:no-privileges]]'); } - await canModifyGroup(socket.uid, data.groupName); return await groups.updateCover(socket.uid, data); }; @@ -353,6 +370,9 @@ SocketGroups.cover.remove = async (socket, data) => { }; async function canModifyGroup(uid, groupName) { + if (typeof groupName !== 'string') { + throw new Error('[[error:invalid-group-name]]'); + } const results = await utils.promiseParallel({ isOwner: groups.ownership.isOwner(uid, groupName), isAdminOrGlobalMod: user.isAdminOrGlobalMod(uid), diff --git a/test/groups.js b/test/groups.js index 986509e863..ee4f91f565 100644 --- a/test/groups.js +++ b/test/groups.js @@ -9,6 +9,8 @@ var db = require('./mocks/databasemock'); var helpers = require('./helpers'); var Groups = require('../src/groups'); var User = require('../src/user'); +var socketGroups = require('../src/socket.io/groups'); +var meta = require('../src/meta'); describe('Groups', function () { var adminUid; @@ -360,6 +362,31 @@ describe('Groups', function () { }); }); + it('should fail if group name is invalid', function (done) { + Groups.create({ name: ['array/'] }, function (err) { + assert.equal(err.message, '[[error:invalid-group-name]]'); + done(); + }); + }); + + it('should fail if group name is invalid', function (done) { + socketGroups.create({ uid: adminUid }, { name: ['test', 'administrators'] }, function (err) { + assert.equal(err.message, '[[error:invalid-group-name]]'); + done(); + }); + }); + + it('should not create a system group', function (done) { + socketGroups.create({ uid: adminUid }, { name: 'mysystemgroup', system: true }, function (err) { + assert.ifError(err); + Groups.getGroupData('mysystemgroup', function (err, data) { + assert.ifError(err); + assert.strictEqual(data.system, 0); + done(); + }); + }); + }); + it('should fail if group name is invalid', function (done) { Groups.create({ name: 'not:valid' }, function (err) { assert.equal(err.message, '[[error:invalid-group-name]]'); @@ -444,6 +471,62 @@ describe('Groups', function () { }); }); + it('should fail to rename if group name is invalid', function (done) { + socketGroups.update({ uid: adminUid }, { groupName: ['updateTestGroup?'], values: {} }, function (err) { + assert.strictEqual(err.message, '[[error:invalid-group-name]]'); + done(); + }); + }); + + it('should fail to rename if group name is too short', function (done) { + socketGroups.update({ uid: adminUid }, { groupName: 'updateTestGroup?', values: { name: '' } }, function (err) { + assert.strictEqual(err.message, '[[error:group-name-too-short]]'); + done(); + }); + }); + + it('should fail to rename if group name is invalid', function (done) { + socketGroups.update({ uid: adminUid }, { groupName: 'updateTestGroup?', values: { name: ['invalid'] } }, function (err) { + assert.strictEqual(err.message, '[[error:invalid-group-name]]'); + done(); + }); + }); + + it('should fail to rename if group name is invalid', function (done) { + socketGroups.update({ uid: adminUid }, { groupName: 'updateTestGroup?', values: { name: 'cid:0:privileges:ban' } }, function (err) { + assert.strictEqual(err.message, '[[error:invalid-group-name]]'); + done(); + }); + }); + + it('should fail to rename if group name is too long', function (done) { + socketGroups.update({ uid: adminUid }, { groupName: 'updateTestGroup?', values: { name: 'verylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstringverylongstring' } }, function (err) { + assert.strictEqual(err.message, '[[error:group-name-too-long]]'); + done(); + }); + }); + + it('should fail to rename if group name is invalid', function (done) { + socketGroups.update({ uid: adminUid }, { groupName: 'updateTestGroup?', values: { name: 'test:test' } }, function (err) { + assert.strictEqual(err.message, '[[error:invalid-group-name]]'); + done(); + }); + }); + + it('should fail to rename if group name is invalid', function (done) { + socketGroups.update({ uid: adminUid }, { groupName: 'updateTestGroup?', values: { name: 'another/test' } }, function (err) { + assert.strictEqual(err.message, '[[error:invalid-group-name]]'); + done(); + }); + }); + + it('should fail to rename if group name is invalid', function (done) { + socketGroups.update({ uid: adminUid }, { groupName: 'updateTestGroup?', values: { name: '---' } }, function (err) { + assert.strictEqual(err.message, '[[error:invalid-group-name]]'); + done(); + }); + }); + it('should fail to rename group to an existing group', function (done) { Groups.create({ name: 'group2', @@ -538,6 +621,20 @@ describe('Groups', function () { }); }); + it('should fail to add user to admin group', async function () { + const oldValue = meta.config.allowPrivateGroups; + try { + meta.config.allowPrivateGroups = false; + const newUid = await User.create({ username: 'newadmin' }); + await socketGroups.join({ uid: newUid }, { groupName: ['test', 'administrators'], uid: newUid }, 1); + const isMember = await Groups.isMember(newUid, 'administrators'); + assert(!isMember); + } catch (err) { + assert.strictEqual(err.message, '[[error:invalid-group-name]]'); + } + meta.config.allowPrivateGroups = oldValue; + }); + it('should fail to add user to group if group name is invalid', function (done) { Groups.join(0, 1, function (err) { assert.equal(err.message, '[[error:invalid-data]]'); @@ -667,11 +764,7 @@ describe('Groups', function () { }); }); - describe('socket methods', function () { - var socketGroups = require('../src/socket.io/groups'); - var meta = require('../src/meta'); - it('should error if data is null', function (done) { socketGroups.before({ uid: 0 }, 'groups.join', null, function (err) { assert.equal(err.message, '[[error:invalid-data]]'); @@ -1012,7 +1105,7 @@ describe('Groups', function () { }); it('should fail to create group if group creation is disabled', function (done) { - socketGroups.create({ uid: testUid }, {}, function (err) { + socketGroups.create({ uid: testUid }, { name: 'avalidname' }, function (err) { assert.equal(err.message, '[[error:no-privileges]]'); done(); }); @@ -1296,7 +1389,7 @@ describe('Groups', function () { it('should fail if user is not logged in or not owner', function (done) { socketGroups.cover.update({ uid: 0 }, {}, function (err) { assert.equal(err.message, '[[error:no-privileges]]'); - socketGroups.cover.update({ uid: regularUid }, {}, function (err) { + socketGroups.cover.update({ uid: regularUid }, { groupName: 'Test' }, function (err) { assert.equal(err.message, '[[error:no-privileges]]'); done(); });