search

Zombies-of-Destruction / PHP_World-of-Zombies

http://game.zodclan-online.de/index.php
0 stars 1 forks source link

Several vulnerabilities concerning SQL / Session Data #1

Closed CodeAequitas closed 11 years ago

CodeAequitas commented 11 years ago

Hey,

while browsing through your code I've found several vulnerabilities concerning SQL Commands. One example is: https://github.com/Zombies-of-Destruction/zod/blob/8342dc004d4c60808f7bf56539c72786a12f2779/site/join.php where you directly execute

$sql_guild = "SELECT * FROM guild_db WHERE guild_id = '" . $_GET['gildenid'] . "'"; $result = mysql_query($sql_guild);

without saniziting or validating your input. Just try to think what happens when an user enters malicious parameters into the get command. I dont know if this is just school project or this is a more serious one, but you should rethink the way you execute your SQL Queries.

If you need help to fix these issues feel free to contact me.

tischler commented 11 years ago

Check out http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php -- it has information on how to prevent this.

If you are wondering how we found you --- http://www.reddit.com/r/programming/comments/1fhm5h/hey_guess_what_time_it_is_again_thats_right/ https://twitter.com/0xabad1dea/status/340964678807216128 https://github.com/search?q=extension%3Aphp+mysql_query+%24_GET&type=Code&s=indexed

Istani commented 11 years ago

Thank you guys.

I will look over the problem and train the other member about that.

Istani commented 11 years ago

Is this the solution? https://github.com/Zombies-of-Destruction/zod/commit/3c1c226e42297b85320dd6934d499b6bc4e1a6e4

CodeAequitas commented 11 years ago

Yea that should do the trick. You may rethink your Database Connection Design and use the PDO Class. Its safer by design and also faster because it allows prepared statements and stored procedures with caching. Since you seem to develop a browsergame, speed and handyness should be important. You may check out http://php.net/manual/en/pdo.prepared-statements.php for more information.

Istani commented 11 years ago

Oh, cool! Thank you very much.

I can not believe that I call myself a programmer, but thats my Job XD I learn PHP all by myself and am really thankfull to someone that show me better ways to programm something.

I think we also should rewrite the functions.php in some classes but i am really not good in OOP and my mate here at github is not a trained programmer...

Well... Thank you very much for everything!

CodeAequitas commented 11 years ago

If you have any questions or discover any problems you cant solve yourself, feel free to contact me. I'm pretty busy with my own stuff, otherwise I could help you out more.