From 70b1bfc953b0feb4338973429b6cacd1102d18a9 Mon Sep 17 00:00:00 2001 From: tabarra Date: Mon, 15 May 2023 18:30:57 -0300 Subject: [PATCH] feat(bot): prevent usage of dangerous permissions --- core/components/DiscordBot/index.ts | 35 +++++++++++++++++++++++++++++ core/webroutes/settings/save.ts | 6 +++++ docs/dev_notes.md | 3 +-- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/core/components/DiscordBot/index.ts b/core/components/DiscordBot/index.ts index e91657655..d3bef1ad7 100644 --- a/core/components/DiscordBot/index.ts +++ b/core/components/DiscordBot/index.ts @@ -197,6 +197,7 @@ export default class DiscordBot { type ErrorOptData = { code?: string; clientId?: string; + prohibitedPermsInUse?: string[]; } const sendError = (msg: string, data: ErrorOptData = {}) => { console.error(msg); @@ -237,6 +238,40 @@ export default class DiscordBot { this.guild = guild; this.guildName = guild.name; + //Checking for dangerous permissions + // https://discord.com/developers/docs/topics/permissions#permissions-bitwise-permission-flags + // These are the same perms that require 2fa enabled - although it doesn't apply here + const prohibitedPerms = [ + 'Administrator', //'ADMINISTRATOR', + 'BanMembers', //'BAN_MEMBERS' + 'KickMembers', //'KICK_MEMBERS' + 'ManageChannels', //'MANAGE_CHANNELS', + 'ManageGuildExpressions', //'MANAGE_GUILD_EXPRESSIONS' + 'ManageGuild', //'MANAGE_GUILD', + 'ManageMessages', //'MANAGE_MESSAGES' + 'ManageRoles', //'MANAGE_ROLES', + 'ManageThreads', //'MANAGE_THREADS' + 'ManageWebhooks', //'MANAGE_WEBHOOKS' + 'ViewCreatorMonetizationAnalytics', //'VIEW_CREATOR_MONETIZATION_ANALYTICS' + ] + const botPerms = this.guild.members.me?.permissions.serialize(); + if (!botPerms) { + return sendError(`Discord bot could not detect its own permissions.`); + } + const prohibitedPermsInUse = Object.entries(botPerms) + .filter(([permName, permEnabled]) => prohibitedPerms.includes(permName) && permEnabled) + .map((x) => x[0]) + if (prohibitedPermsInUse.length) { + const name = this.#client.user.username; + const perms = prohibitedPermsInUse.includes('Administrator') + ? 'Administrator' + : prohibitedPermsInUse.join(', '); + return sendError( + `This bot (${name}) has dangerous permissions (${perms}) and for your safety the bot has been disabled.`, + { code: 'DangerousPermission' } + ); + } + //Fetching announcements channel if (this.config.announceChannel) { const fetchedChannel = this.#client.channels.cache.find((x) => x.id === this.config.announceChannel); diff --git a/core/webroutes/settings/save.ts b/core/webroutes/settings/save.ts index 520d48bed..ab924f696 100644 --- a/core/webroutes/settings/save.ts +++ b/core/webroutes/settings/save.ts @@ -435,6 +435,12 @@ async function handleDiscord(ctx: Context) { - **Wrong guild/server ID:** read the description of the guild/server ID setting for more information. - **Bot is not in the guild/server:** you need to [INVITE THE BOT](${inviteUrl}) to join the server. - **Wrong bot:** you may be using the token of another discord bot.`; + } else if (errorCode === 'DangerousPermission') { + extraContext = `Please keep in mind that: + - These permissions are dangerous because if the bot token leaks, an attacker can cause permanent damage to your server. + - You need to remove the permissions listed above to be able to enable this bot. + - No bot should have more permissions than strictly needed, specially \`Administrator\`. + - You should never have multiple bots using the same token, create a new one for each bot.`; } return ctx.send({ type: 'danger', diff --git a/docs/dev_notes.md b/docs/dev_notes.md index 8c90760dc..1dbe66ec3 100644 --- a/docs/dev_notes.md +++ b/docs/dev_notes.md @@ -106,6 +106,7 @@ - [x] check/merge redm vehicle boost > beta2 release +- [x] bot should check if it has any dangerous permission - [ ] inject consts isZapHosting and isPterodactyl in ctxUtil - [ ] stats: - [ ] adapt the new runtime specs, separate temp stats from classic stats @@ -113,8 +114,6 @@ - [ ] add isPterodactyl to stats - [ ] start tracking the ban search duration - [ ] jwe (in header?) -- [ ] bot should check if it has any dangerous permission - - message should also inform the user that multiple bots on the same token is a terrible idea - [ ] maybe add some debug logging to `AdminVault.checkAdminsFile()`, to find out why so many people are having issues with their logins - maybe even add to the login failed page something like "admin file was reset or modified XXX time ago" - [ ] Add a tracking for % of redm/fivem/libertym servers to txTracker