Grab IP Address From URL - Prevent SQL Injection

[Mko]

I have the following code ($c2 is my connection variable):

$host = $_GET['host'];
$loginQuery = mysql_query("SELECT * FROM sessions WHERE hostname LIKE '". $host ."' ORDER BY id DESC", $c2) or print(mysql_error());

 

In the URL, someone were to put host=127.0.0.1', they would have an error message spit out to them (something along the lines of: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''127.0.0.1'' ORDER BY id DESC' at line 1), indicating a SQL Injection exploit.

 

How would I go about fixing this, and also preventing SQL Injection?

 

Thanks a bunch,

Mark

[xyph]

Use mysql_real_escape_string. Keep in mind, this is only helpful if you QUOTE your values within the query, which you've done.

 

If you're dealing with integers, use typecasting instead

 

$query = 'SELECT columns FROM table WHERE id = '.(int)$_GET['id'];

 

This will force the value to be an integer, 0 if it can't properly be converted. There's more in the manual/online on typecasting if you're interested.

[Psycho]

You need to validate/sanitize all user input for the TYPE of value that you expect. In your previous post on almost the exact same problem you were showed how to force a value to be an integer. In this case, I *assume* you are storing the IP addresses as strings. For ANY value that should be a string, when using it in a query you will want to run it through mysql_real_escape_string() to prevent SQL Injection. But, in this case I would also suggest adding validation to ensure the value entered at least is something like a valid address before even running a query on it.

 

Also, as an aside, you might consider storing your IP addresses as integers as well. You would just convert the IP from  xxx.xxx.xxx.xxx to an integer using ip2long to convert to an integer. More info here: http://daipratt.co.uk/mysql-store-ip-address/

[xyph]

You need to validate/sanitize all user input for the TYPE of value that you expect. In your previous post on almost the exact same problem you were showed how to force a value to be an integer. In this case, I *assume* you are storing the IP addresses as strings. For ANY value that should be a string, when using it in a query you will want to run it through mysql_real_escape_string() to prevent SQL Injection. But, in this case I would also suggest adding validation to ensure the value entered at least is something like a valid address before even running a query on it.

 

In a perfect world, you sanitize all values as strictly as possible.

 

If this column were to later contain desired values that don't fit this strict pattern though, you'd have to change code to make it work.

 

I don't see an issue with only use real_escape on quoted SQL strings in cases like this, where a valid hostname could be a local domain, or a top-level domain, etc. An attempt at injection would result in 0 rows returned, a worst case scenario would be a new attack vector that gets by real_escape, something that would screw nearly every MySQL-driven PHP application that exists. Regardless, you should be performing a strict validation or cleanse before using or outputting the data, making the initial strict comparison a little redundant.

 

Just food for thought. There's nothing *wrong* at all with your method.

[Psycho]

You need to validate/sanitize all user input for the TYPE of value that you expect. In your previous post on almost the exact same problem you were showed how to force a value to be an integer. In this case, I *assume* you are storing the IP addresses as strings. For ANY value that should be a string, when using it in a query you will want to run it through mysql_real_escape_string() to prevent SQL Injection. But, in this case I would also suggest adding validation to ensure the value entered at least is something like a valid address before even running a query on it.

 

In a perfect world, you sanitize all values as strictly as possible.

 

If this column were to later contain desired values that don't fit this strict pattern though, you'd have to change code to make it work.

 

I don't see an issue with only use real_escape on quoted SQL strings in cases like this, where a valid hostname could be a local domain, or a top-level domain, etc. An attempt at injection would result in 0 rows returned, a worst case scenario would be a new attack vector that gets by real_escape, something that would screw nearly every MySQL-driven PHP application that exists. Regardless, you should be performing a strict validation or cleanse before using or outputting the data, making the initial strict comparison a little redundant.

 

Just food for thought. There's nothing *wrong* at all with your method.

 

Right, I was trying to convey that mysql_real_escape_string() was a necessity and validation of the format was only a suggestion - which was based on what he explicitly provided. But, I agree with your response.