Discord bots, just like web servers, deal with untrusted data. Most of those bots implement interactive commands, which makes the attack surface significantly larger. This guide is to improve security of Discord bots.
Security is usually represented by the three elements of CIA (not related to the agency) - Confidentiality, Integrity, and Availability.
- Confidentiality: Protecting your secrets (and others' secrets) from attackers. "Secrets" include your bot token, message contents and other PII, files stored on your computer and other private data.
- Integrity: Protecting your data from manipulation by attackers. "Manipulation" can vary from compromising the host computer to improper permissions/roles assignment or data spoofing/tampering (e.g. cheating in a chess game to obtain points).
- Availability: Protecting your bot from crashes or similar situations. Examples include infinite loop, blocking codes and error handling issues.
Here are some tips to improve security of your bots:
discord.py is regularly updated to reflect changes to Discord API. Older versions of discord.py might contain bugs, which can cause crashes. You should also update downstream components (operating system and Python mainly) regularly, as well as any dependencies that your bots use. Here are some common ones:
- Pre-made bot framework like Red-DiscordBot has been subject to some remote code execution flaws.
- Extensions of discord.py, such as jishaku.
- Other dependencies that are frequently used by discord.py bots: FFmpeg, youtube-dl, Pillow.
Also remember to update Discord (and modded clients, if any) regularly.
If you are sharing the code on GitHub, you may receive pull requests. Make sure to review the changes before merging, so that viruses cannot slip through.
2FA (2-Factor Authentication) makes it harder to compromise your account. (See a video by Tom Scott for how it works.) Two-factor authentication does not require a phone - as long as you have a 2FA code generator app, it will work. See how to enable 2FA on Discord and on GitHub.
Do not use 2FA over SMS as this is rather easy to bypass (by scamming the phone company).
Typosquatting is an attack that abuses the fact that humans make typo. For example, if you accidentally typed pip install -U discor.py instead of pip install -U discord.py, you may install a virus called discor.py. While PyPI will delete malicious packages if reported, it's likely to stay for a few days infecting other computers.
Don't host on your friend's computer without making sure they're not evil. Same goes for any website that claims to be free; there will always be a catch.
When generating a invite link, do not ask permissions that are unnecessary or only used in optional features. Never ask Administrator.
Tokens often leak because the developer has put it in a file and shared the file. Tokens should be stored in an environment variable, not a file. For Windows, this can be done by systempropertiesadvanced.exe. On Linux systems, type export BOT_TOKEN="token" before running the bot. In your Python code, call bot.run(os.environ["BOT_TOKEN"]) to run - which retrieves the token from the environmental variable.
If you, for some reason, cannot set environment variables, create a file named .env like this:
BOT_TOKEN="whatever"
Also add .env to .gitignore. You should then install python-dotenv via pip, and add this to the top of your code.
from dotenv import load_dotenv
load_dotenv()After that, you can access os.environ just like normal. You should use this for any tokens used by the bot, not just Bot Tokens.
You should not pass tokens as command-line arguments to prevent leakage in multi-user setup on Linux. (This is common in shared hostings.)
GitHub Secret Scanning detects bot tokens on public GitHub repositories, notifies Discord and invalidates the token. Therefore, if you spotted a Discord token leaked somewhere, upload it on GitHub public repositories. This sounds counter-intuitive but this works.
Don't implement eval yourself, unless you know how to do things safely. jishaku will handle this for you.
Add @commands.is_owner() before any dangerous commands, such as shutdown or eval. This makes sure only bot owners can call the command.
@bot.command()
@commands.is_owner()
async def eval(ctx, *, code):
await ctx.send(eval(code))First, do not use this to protect you from attacks. This is to protect servers from attacks. If you need to protect yourself, use is_owner.
You should always check permissions of a user when invoking privileged actions, such as deleting messages or banning someone. To check permissions in the current channel, use has_permissions.
@bot.command()
@commands.has_permissions(manage_messages=True)
async def purge(ctx, count: int = 10):
await ctx.channel.purge(limit=count)To check permissions in other channels, use abc.GuildChannel.Channel.permissions_for. This is necessary when checking for permissions in voice channels. Here is an example:
@bot.command()
@commands.guild_only()
async def force_disconnect(ctx, member: discord.Member):
if not (member.voice and member.voice.channel):
return await ctx.send("Already disconnected")
if not member.voice.permissions_for(ctx.author).move_members:
return await ctx.send("Forbidden")
await member.move_to(None)If you are implementing a feature to manage users' roles or to kick/ban someone, you should compare the performer's top role and the target's top role. For example, the code below is problematic, as anyone with Ban Members permission can ban any user, including those who they cannot ban normally.
# BAD CODE
@bot.command()
@commands.has_permissions(ban_members=True)
async def insecure_ban(ctx, member: discord.Member):
await member.ban()This code looks fine, but will allow privilege escalation in the following case:
- There are three roles, ordered by the hierarcy: Admin, Bot, Helper
- Helpers can ban members, but Discord does not allow helpers to ban each other.
- However, since bots' top roles are above the helpers, bots can ban any helpers.
- A helper can use
insecure_bancommand to ban another helper.
To prevent this, an additional role check must be added:
# BAD CODE
@bot.command()
@commands.has_permissions(ban_members=True)
async def insecure_ban(ctx, member: discord.Member):
if ctx.author.top_role > member.top_role:
await member.ban()Here are some web security advices that apply to Discord bots.
First, consider if it is necessary. While there are certain operations like Git or FFmpeg that do require command-line, some has Python alternatives that you can use instead.
Second, use asyncio.create_subprocess_exec or asyncio.subprocess.Process with shell=False. This will greatly reduce attack surface. Do not use os.system, regular subprocess or any other methods as they are blocking (see below).
To use create_subprocess_exec:
import asyncio
# NOTE: The example below is *still dangerous* due to how curl works. See "Server-Side Request Forgery" section below.
proc = await asyncio.create_subprocess_exec("/usr/bin/curl", "--", url, stdout=asyncio.subprocess.PIPE)
stdout, _ = await proc.communicate()
result = stdout.decode("utf-8")Note the use of --. You should always pass -- to prevent unintended arguments (e.g. url = "--version") from being added.
When storing a file, never use filenames from attacker-controllable places, such as attachment filenames, usernames or channel names. Instead, generate a Version 4 UUID (or any other randomly generated, collision-safe ID) and store the ID somewhere. Do not trust file extensions - strip if you can. Example of UUID-based storage:
import uuid
@bot.command()
async def do_something_with_file(ctx, *, note):
try:
file = ctx.message.attachments[0]
except IndexError:
return
if file.size > 10 * 1024 * 1024: # 10MiB
return
fid = str(uuid.uuid4())
# record in database
record_file_data(ctx.author.id, note, file.filename, fid)
await file.save(f"./files/{fid}")Do not use filenames as user-facing identifiers; instead, make a dictionary of known filenames and its IDs. This prevents attacks such as path traversal and denial of service by reading CON (stdin) on Windows.
mobs = {
"creeper": "./mob/creeper.png",
"zombie": "./mob/zombie.png"
}
@bot.command()
async def mob(ctx, name):
try:
filename = mobs[name]
except KeyError:
return
await ctx.send(f"Here's {name}!", file=discord.File(filename, "mob.png"))You should also make sure that the downloaded file is not too large. Attachment.size can be compared for this purpose.
When using inputs in a SQL query, use a prepared statement:
# Note that you pass a tuple, even if there is only one item:
await cur.execute("INSERT INTO people VALUES (?)", ("apple502j",))
# Alternatively, you can use a dict:
await cur.execute("SELECT * FROM people WHERE name=:name", {"name": "apple502j"})This prevents SQL injection, or "Bobby Table" attack, which can happen if you just use substitutions:
# INSECURE CODE!
name = '"pwned"); DROP TABLE people; --'
await cur.execute(f"INSERT INTO people VALUES ({name})")
# Oh no, all data gone forever.Never trust URL, especially if you are the one that is accessing it. If you need to access attacker-controlled URL (e.g. passed as command argument):
- Check the protocol. For example, while
file:///etc/passwdis a valid URL, you probably don't want to access these - in the worst case you'll end up leaking your files. Make sure the protocol ishttps. (WhilehttpURL is safe to access, it does not have protection against person-in-the-middle eavesdropping on your connection. See "Person-in-the-Middle Vulnerabilities" section below.) - Check the origin. Accessing private IP addresses can cause other devices in your network to operate in an unexpected manner - or, if you are hosting on cloud, can leak your credentials. Your code should not access private IPs and
localhost.
To check the validity of an URL, you can use urllib.parse:
from urllib.parse import urlparse
import re
SUFFIX = re.compile(r"\.[a-z]+$")
def url_valid(url):
o = urlparse(url)
return o.scheme in ("http", "https") and hostname_valid(o.hostname)
def hostname_valid(hostname):
return SUFFIX.match(hostname)
url_valid("file:///etc/passwd") # False
url_valid("http://192.168.1.12:8080/factory_reset") # FalseYou should also disable redirects when connecting (e.g. session.get(url, allow_redirects=False) to prevent bypass.
Never use str.format on untrusted string. Using str.format on untrusted string can leak global variables, tokens, and in some rare cases, execute code. Similarly, never nest str.format.
For example, in the code below, running $print {0._state.http.token} can leak the token.
# BAD
@bot.command()
async def print(ctx, *, fstr):
await ctx.send(fstr.format(ctx.author))Here is an example of nested str.format call. This can be exploited by setting a nickname {0._state.http.token} to leak the token.
# BAD
@bot.command()
async def print(ctx):
s = "Hello, {}".format(ctx.author.display_name)
# Nested str.format
await ctx.send(s.format(ctx.author))You must not use pickle to load untrusted data (e.g. attachments). Loading a pickle can cause various side effects, such as executing commands Use JSON instead.
When connecting to an external website, make sure nobody is eavesdropping on your connection. In many cases this is as simple as using https:// instead of http://. If you are somehow using WebSocket, use wss:// instead of ws://.
Some programs allow disabling certificate verification. This nullifies the protection provided by HTTPS. Certificates should always be verified. For example, when using youtube-dl, nocheckcertificate should never be manually set (the default behavior is secure).
Any functionality of a bot uses variable amount of system resource, usually CPU and memory, and sometimes disk and network. Excessive consumption of those resources can result in instability, and potential additional costs (depending on the server hosts). Additionally, crashes, deadlocks and so-called "blocking codes" can all result in service downtime. For this reason, availability is just as important as other components of the CIA.
Flooding requests is one easy way to perform DoS attack. Adding @commands.cooldown() check to commands that perform expensive operations can prevent excessive use of those commands. Use BucketType.user, as other checks can be bypassed or abused rather easily; the default value, BucketType.default, is applied globally and prevents other, legitimate users from using the command.
@bot.command()
@commands.cooldown(1, 20, commands.BucketType.user)
async def do_expensive_operaration(ctx):
# do something
...If the cooldown duration is short (e.g. less than a minute), it's not worth sending a "cooldown failed" message - sending a "cooldown failed" message can be expensive, after all.
When using cooldowns, either do not set cooldown_after_parsing or set cooldown_after_parsing=False on @bot.command(). Parsing is expensive, especially when using API-powered converters.
When an operation takes time, there are two options:
- Blocking: your bot should not perform any operations. No responding, no heartbeat. Your bot will play dead, until your operation finishes.
- Asynchronous: other parts of your bot can continue operations. Asynchronous operations are marked by the
awaitkeyword, as seen inawait ctx.send().
Blocking code is not always bad; there are many situations where the operation takes so little time. However, if the operation takes some time - say, a few seconds, then your bot will be "dead" for that duration. For example, time.sleep() is an obvious blocking function - your entire bot will sleep. If you just want a part of your bot, for example a timer command to sleep, use an asynchronous variant, await asyncio.sleep().
# Bad - your bot will be dead for a minute.
time.sleep(60)
# Good - your bot will be responsive as usual.
await asyncio.sleep(60)IO is another kind of operations that usually take time. If you are making a request over the internet, use aiohttp instead of urllib.request or requests:
# Bad
resp = requests.get("https://example.com/")
text = resp.text
# Good
async with aiohttp.ClientSession() as cs:
async with cs.get("https://example.com/") as resp:
text = await resp.text()If your bot is small (and you have a good SSD), file IO is not usually a problem. Otherwise, aiofiles is a good asynchronous file IO library. For database operations, use asqlite or asyncpg.
For invoking subprocesses, use asyncio.subprocess.
Regular expression is a great way to check if a string matches certain conditions. However, it can also be used to hang the bot (aka ReDoS attack). You can check if your regular expression is exploitable by regexploit tool: pip install regexploit, then run regexploit and paste your regular expression. If it is vulnerable, you should rewrite your regular expression. For example, regexploit shows that the regular expression ^(\w+| )+\.?$ is vulnerable below.
Welcome to Regexploit. Enter your regexes:
^[\w-]{3,20}$
No ReDoS found.
^((?:https?:\/\/)?(?:[\w-]+\.)+(?:xn--[a-zA-Z\d]+|[a-zA-Z]{2,})(?:\/[^\s"<>\\^`{|}]*)?)$
No ReDoS found.
^(\w+| )+\.?$
Pattern: ^(\w+| )+\.?$
---
Redos(starriness=11, prefix_sequence=SEQ{ }, redos_sequence=SEQ{ [WORD]{1+}{1+} [2e:.]{0,1} $[2e:.] }, repeated_character=[WORD], killer=None)
Worst-case complexity: 11 (exponential)
Repeated character: [WORD]
Example: '0' * 3456
If there is a chance a code can error, you must handle the error. "Handling an error" can mean several things:
- Showing error message, if it's users' fault (e.g. missing permissions).
- Using fallback methods.
- Logging.
- Ignoring, aka
except: pass. This option should only be used when the result of the operation does not matter, or when other methods don't work. Specifically, you should ignoreCommandNotFound,DisabledCommand, andCommandOnCooldownfor short-duration cooldowns.
Discord server will ask the client to sleep before making a new request if the client makes too many requests. Bots should not use APIs with heavy (>1h) rate limits - editing channel name/topic and adding emojis - without appropriate cooldowns. Note that those cooldowns should be guild-wide.
Some servers, namely repl.it, share IP address between hundreds of clients using discord.py. Such environment can cause IP-address based rate limiting to trigger, making it impossible to perform any API call. You should switch your host if this is the case.
Users should not be able to read data on guilds they are not in. However, some features in discord.py makes it easy to expose those kinds of information easy to expose. Bots should be designed with this in mind.
Global lookup (lookup by methods of Client/Bot) can return results from any guild. If you do not know where the ID is coming from, you should lookup using methods of known Guild instead.
Converters use global lookup on DM. To prevent users from extracting information on other guilds, you should either:
- Disable commands on DM (
@commands.guild_only()) - Make sure the user has access to the guild/channel/member
# Approach 1
@bot.command()
@commands.guild_only() # This ensures local guild lookup
async def member_info(ctx, member: discord.Member):
await ctx.send(f"{member} ({member.id}) - nickname: {member.nick}")
# Approach 2
@bot.command()
async def member_info(ctx, member: discord.Member):
if not member.guild.get_member(ctx.author.id):
return
await ctx.send(f"{member} ({member.id}) - nickname: {member.nick}")discord.Message converter allows referencing messages on other guilds or on channels the author cannot see. You should always make sure the author can see the resolved message.
@bot.command()
async def read(ctx, msg: discord.Message):
if not msg.channel.permissions_for(ctx.author).read_message.history:
return
await ctx.send(f"{member} ({member.id}) - nickname: {member.nick}")discord.Guild converter allows referencing any guild. It's pretty unlikely this feature is needed.
Bot data has to be in a consistent state. For example, there should not be a way to corrupt save data, or store unexpected data.
JSON files can be easily corrupted by concurrent write access. For example,
# Bad
@bot.commands()
async def save_data(ctx):
with open("./data.json", "r", encoding="utf-8") as fp:
o = json.load(fp)
msg = await bot.wait_for("message", check=lambda msg: msg.channel == ctx.channel and msg.author == ctx.author)
o[ctx.author.id] = msg.content or ""
with open("./data.json", "w", encoding="utf-8") as fp:
json.dump(o, fp)If two users use the command at the same time, one of the user's changes will be lost. You should use SQL if concurrent write access is required.
wait_for event, just like other event handlers, listens to any events of the type by default. For example, when listening to message event, it can resolve messages from different user or in different channels. While technically optional, check argument should always be passed.
timeout argument is also important, because it allows the code to exit without waiting for the condition to occur forever. For example, in a bot that allows users to play up to one game at once, you could cause deadlock by deleting a channel hosting a game if timeout is missing or is too long. You should catch asyncio.TimeoutError from wait_for, do cleanups and return.
You should set allowed_mentions parameter in Client/Bot constructor to prevent accidental mentioning of roles or @everyone. AllowedMentions.none() will disable any mentions by default - which can be overridden on individual calls to send().
While there is not much of a security impact, you should escape Markdown inside message contents or embeds, just so that you don't accidentally rickroll someone.
@bot.command()
async def say(ctx, *, message: commands.clean_content(escape_markdown=True)):
embed = Embed(title="Hi!", description=message)
await ctx.send(embed=embed)
Amazing! Thank you for sharing!