feat(bot): prevent usage of dangerous permissions

citizenfx · May 15, 2023 · 70b1bfc · 70b1bfc
1 parent a740a7b
commit 70b1bfc
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 2 deletions.
diff --git 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
@@ -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
@@ -106,15 +106,14 @@
 - [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
     - [ ] add bot enabled / whitelist back into stats
     - [ ] 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