Sanitising user input for MySQL

Question

Possible Duplicate:
The ultimate clean/secure function

I have run into a few problems with some of my coding, I have unwillingly uncovered a possible sql injection in my script and I can't seem to resolve the issue, so I wondered if anyone from around here could help out with it.

Can I just say I am fine coding my scripts, its just I have absolutely no idea how database injections happen, how they work or how to prevent them, however it is quite clear my cleaning function does not work as intended.

Here is the offending chunk of code, when I add the following after id it will spit out a MySQL error, which I have read alot about that meaning my code is vulnerable.

&id=369';

So just adding a comma and a semi-colon breaks the MySQL query, that isn't desired. Here is the code that handles the query and displays the data;

$id = $this->clean($_GET['id']);
#$id = filter_input(INPUT_GET, 'id', FILTER_SANITIZE_STRING);
$pm_query = mysql_query("SELECT * FROM `staff_pm` WHERE `id` = '{$id}' AND `status` IN(0, 1)") or die(''.mysql_error());

So I have tried one of the integrated functions within PHP but that spits out the same sql error, this is my cleaning function which I was told beforehand was fine:

function clean( $str )
{
return mysql_real_escape_string( htmlspecialchars( trim( $str ) ) );
}

Just really wanted to know your opinions on the matter, it is a constant worry that my coding will be taken advantage of by a person who is looking for these types of vulnerabilities, and I really want to make sure it is up to scratch and learn some techniques to better protect it.

I have been told to convert to PDO, however it seems like a large job converting and I will wait until I come to re-creating the application before I do that, where I am going to go for a much more object-orientated approach.

The SQL error I receive is this:

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 '\'; AND status IN(0, 1)' at line 1

Thank you for any help, I really do appreciate it.

Regards

Don't use htmlspecialchars for database insertion, just do the escape_string. Alwasy try to store data in "raw" format, and do an appropriate conversion when you retrieve the data, when you KNOW what the data will be used for. If you pull it out and use it in a non-web context, you'd have to undo the htmlspecialchars and waste cycles. — Marc B, Aug 02 '12 at 14:22
@MarcB thank you for your comment, I really appreciate it! I completely understand and agree with what you have stated, I will store everything in the database raw and then sanitize it as and when I need it should I be displaying it. Thank you! — Jake Ball, Aug 02 '12 at 14:24

score 2 · Answer 1 · edited May 23 '17 at 12:02

2

What this guy said: use prepared statements and parameterized queries.

How can I prevent SQL injection in PHP?

edited May 23 '17 at 12:02

Community

1
1

answered Aug 02 '12 at 14:16

sent1nel

1,580
1
15
31

Hello, Looks interesting I will definitely be taking a read into the whole thread as it is something I really need to work on, thanks! – Jake Ball Aug 02 '12 at 14:23

Mark Byers · Accepted Answer · 2012-08-02T14:20:19.040

1

I'd also recommend not using the obsolete mysql_* functions and instead use PDO.

But to fix your existing code, use intval.

$id = intval($_GET['id']);

edited Aug 02 '12 at 14:20

answered Aug 02 '12 at 14:15

Mark Byers

811,555
193
1,581
1,452

Thanks Mark, yes I definitely want to move away from mysql because a lot of the attacks are focused on taking advantage of it, and also I am going for a focus on a much more object-orientated focus to make it look and feel much cleaner. Thanks! – Jake Ball Aug 02 '12 at 14:19
**Do not use** `mysql_query` in new applications. It's way more trouble than it's worth, as you've just discovered. It's dangerous by default, where `mysqli` and PDO are safe by default if used properly. – tadman Aug 02 '12 at 14:56

score 1 · Answer 3 · answered Aug 02 '12 at 14:17

1

To me it looks like your id column is of type integer and you're trying to compare it to '369\''?

Comparing the id column to '369' is valid (in mysql at least), but comparing an integer to a non-integer string isn't.

answered Aug 02 '12 at 14:17

Matthew

24,703
9
76
110

Thank you for your comment, yes I am using the id column as an int, and I now understand why it was throwing a MySQL error. Thanks! – Jake Ball Aug 02 '12 at 14:22

Sanitising user input for MySQL

3 Answers3