SQL Injection

[emeth-]

Hey Nadya!

You have this api call: /online_game/public/getnickname.php?playernum=1

Which receives a GET parameter here: https://github.com/nprimak/hnefatafl/blob/master/online_game/public/getnickname.php#L6

Which passes $_GET["playernum"] to find_nickname_by_gameid() as the second parameter.

That GET parameter then gets passed directly to the database query here, without being escaped: https://github.com/nprimak/hnefatafl/blob/master/online_game/private/functions.php#L110

This api call: /online_game/public/getnickname.php?playernum=1 Is expected to execute a query like this: SELECT nickname FROM player WHERE game_id = 1 AND player1 = 1

However, the user can pass data that gets added to the sql command directly via the url, like so: /online_game/public/getnickname.php?playernum=1 OR player1>0 Which turns into SELECT nickname FROM player WHERE game_id = 1 AND player1 = 1 OR player1>0

The solution to this is both typecasting and escaping.

Note that you correctly typecast the ID variable $id = (int) $id; but didn't do so with the other variable here: https://github.com/nprimak/hnefatafl/blob/master/online_game/private/functions.php#L105

If user input is not a number, but rather a string variable you're attempting to pass to the database, you can use mysql_real_escape_string to sanitize it (you have it wrapped here: https://github.com/nprimak/hnefatafl/blob/master/online_game/private/functions.php#L8-L14).

Note that PHP has also been moving away from this kind of approach in general, and is promoting the use of prepared statements for DB interaction now too: http://php.net/manual/en/pdo.prepared-statements.php

Anyway, not a big deal - just letting you know in case you found it interesting to learn.

A friend and I played this game today on your app, and enjoyed it. Fantastic job, keep on shipping!

Best, emeth

[nprimak]

Thanks Emeth!

I really appreciate the feedback, and for bringing this to my attention. There are few things better than hearing that someone enjoyed something I built, so thank you for making my day brighter. I will definitely try to fix these problems ASAP.

On Wed, Nov 11, 2015 at 4:52 PM, emeth- notifications@github.com wrote:

Hey Nadya!

You have this api call: /online_game/public/getnickname.php?playernum=1

Which receives a GET parameter here:

https://github.com/nprimak/hnefatafl/blob/master/online_game/public/getnickname.php#L6

Which passes $_GET["playernum"] to find_nickname_by_gameid() as the second parameter.

That GET parameter then gets passed directly to the database query here, without being escaped:

https://github.com/nprimak/hnefatafl/blob/master/online_game/private/functions.php#L110

This api call: /online_game/public/getnickname.php?playernum=1 Is expected to execute a query like this: SELECT nickname FROM player WHERE game_id = 1 AND player1 = 1

However, the user can pass data that gets added to the sql command directly via the url, like so: /online_game/public/getnickname.php?playernum=1 OR player1>0 Which turns into SELECT nickname FROM player WHERE game_id = 1 AND player1 = 1 OR player1>0

The solution to this is both typecasting and escaping.

Note that you correctly typecast the ID variable $id = (int) $id; but didn't do so with the other variable here:

https://github.com/nprimak/hnefatafl/blob/master/online_game/private/functions.php#L105

If user input is not a number, but rather a string variable you're attempting to pass to the database, you can use mysql_real_escape_string to sanitize it (you have it wrapped here: https://github.com/nprimak/hnefatafl/blob/master/online_game/private/functions.php#L8-L14 ).

Note that PHP has also been moving away from this kind of approach in general, and is promoting the use of prepared statements for DB interaction now too: http://php.net/manual/en/pdo.prepared-statements.php

Anyway, not a big deal - just letting you know in case you found it interesting to learn.

A friend and I played this game today on your app, and enjoyed it. Fantastic job, keep on shipping!

Best, emeth

— Reply to this email directly or view it on GitHub https://github.com/nprimak/hnefatafl/issues/1.

Nadya Primak Website: www.nadyaprimak.com Cell: (507) 226-2553