ddc / DiscordBot

A Bot for Discord
https://ddc.github.io/DiscordBot
MIT License
2 stars 2 forks source link

insert_default_initial_server_configs fails if server name contains single quotes #1

Closed esauvisky closed 4 years ago

esauvisky commented 4 years ago

If the server name contains single quotes, the bot fails while trying to insert the entry for the server into the recently created db. This happens both with sqlite3 and postgresql.

bot    | [Jun/24/2020 23:49:46]:[ERROR]:[connection.py:execute:48]:PostgreSQL
bot    | Traceback (most recent call last):
bot    |   File "/opt/DiscordBot/src/databases/postgres/connection.py", line 39, in execute
bot    |     await conn.execute(sql)
bot    |   File "/usr/local/lib/python3.8/site-packages/asyncpg/connection.py", line 272, in execute
bot    |     return await self._protocol.query(query, timeout)
bot    |   File "asyncpg/protocol/protocol.pyx", line 316, in query
bot    | asyncpg.exceptions.PostgresSyntaxError: syntax error at or near "s"
bot    | [Jun/24/2020 23:49:46]:[ERROR]:[connection.py:execute:49]:Sql:(INSERT INTO servers (discord_server_id, server_owner_id, server_name, region, icon_url)
bot    |                       VALUES (
bot    |                       000000000000000000,
bot    |                       000000000000000000,
bot    |                       'Emi's testing server',
bot    |                       'brazil',
bot    |                       '');
bot    |                       INSERT INTO server_configs (discord_server_id) VALUES (000000000000000000);)

In servers_sql.py:18-31:

    async def insert_default_initial_server_configs(self, servers: discord.Guild):
        databases = Databases(self.bot)
        for server in servers:
            icon_url = str(server.icon_url)
            current_server = await self.get_server_by_id(server.id)
            if len(current_server) == 0:
                sql = f"""INSERT INTO servers (discord_server_id, server_owner_id, server_name, region, icon_url)
                      VALUES (
                      {server.id},
                      {server.owner_id},
                      '{server.name}',      <-----------
                      '{server.region}',
                      '{icon_url}');
                    INSERT INTO server_configs (discord_server_id) VALUES ({server.id});"""
                await databases.execute(sql)

I didn't look yet at the other files, but if lack of sanitization as above is a common standard over the queries, it might need attention as this potentially creates an attack vector for SQL injection, particularly if there's any user input that gets sent to the db straightaway.

ddc commented 4 years ago

Issue with single quotes has been fixed on version 1.41 As I can tell, you pretty much forced the error by looking at all those zeros from server id

About the sql injection on bots in discord realms, theres no need to use alchemy: Its all about commands. If the message starts with !play, then isn't gonna kick anyone. The only exception to this would be an eval command. As in, commands that take a string and then execute that string as program code. Usually eval commands are restricted to certain people (like the mods), but if you found a bot that has a publicly usable eval command, then yeah, you could do make it do pretty much anything. But with other, regular commands? No way. If it's an open eval command that runs on the bot host itself without any environment trickery, yeah, that's a problem. But if it's just a command that passes stuff over to ideone or is restricted to only the bot owner there shouldn't be any harm.

If you want to know more about discord bots i can recommend some reading: https://discordpy.readthedocs.io/en/rewrite/api.html https://github.com/Rapptz