TrueMB / DiscordNotify

Connects your Discord Server with your Minecraft Server!
https://www.spigotmc.org/resources/discordnotify-notifications-and-utils-for-discord.94230/
GNU General Public License v2.0
7 stars 6 forks source link

fix: potential sql injection #16

Closed LittleBigBug closed 10 months ago

LittleBigBug commented 10 months ago

use prepared statements to prevent possibility of sql injection

TrueMB commented 10 months ago

Hey, thank you very much for the info! This is definitly the better way handling it. But SQL Injections weren't possible before, since the values are checked before using it in the sql.

ByteZ1337 commented 10 months ago

https://github.com/psp1g/DiscordNotify/commit/878a60101040a9ca2d7f82c7275699ad360aba47 you can cherry-pick this commit as well if you want (or I can PR it). Didn't see there's another class with SQL statements. And where are the values checked? Couldn't find it in the code.

TrueMB commented 10 months ago

SQL injections are as far as I know only really possible with String inputs. Most are done by the system or are Enums: https://github.com/TrueMB/DiscordNotify/tree/main/src/main/java/me/truemb/discordnotify/enums

The Player Name is checked by mojang/microsoft and can only contain letters and numbers.

EDIT: Also for you information, I plan on doing some updates as well. After that I will do an update release. Once again thanks for both pr!

LittleBigBug commented 10 months ago

SQL injections are as far as I know only really possible with String inputs. Most are done by the system or are Enums: https://github.com/TrueMB/DiscordNotify/tree/main/src/main/java/me/truemb/discordnotify/enums

The Player Name is checked by mojang/microsoft and can only contain letters and numbers.

EDIT: Also for you information, I plan on doing some updates as well. After that I will do an update release. Once again thanks for both pr!

Still best practice to use prepared statements anyways. People could be (unfortunately) using it for cracked, but it would still be bad to have that around.

Definitely! We have a few other features/fixes that I could cherry pick into more PRs if you'd like :)

ByteZ1337 commented 10 months ago

java_lVzUgGEQQQ Yeah, just checked again to make sure. You could break (and most likely inject into) the statements on a cracked server. Mojang's valid username check sadly still allows this kind of stuff on cracked servers.