MySQL injection attacks in php - question?

PHP
1

Hello All,

Have a quick question about MYSQL injection attacks and how to use $_GET.

For example, say I had a sign-up form on an email where the user clicked this link:

signup.php?id=12345

Then on this signup.php page the following query was executed using $_GET:


UPDATE tbl SET live = '1' WHERE id = {$_GET['id]}

Now a hacker could be clever and go into the link as:

page.php?id=12345’ OR 1;–

Which then changes the query on the site to


UPDATE tbl SET live = '1' WHERE id = '12345' OR 1;--'

Which changes the ‘live’ column in all database entries to = 1

So my question is how can I get around something like this and stop hacking this way with a process like this?

Thanks

2

PHP has a neat little function. :wink:


if(is_int($_GET['id']){
  UPDATE tbl SET live = '1' WHERE id = $id;
 // if successfull echo yay 
} else { echo 'bad id';}
3

Hi Ryan,

Yes, thanks. Was getting a bit confused myself but thanks for everyone’s help.

Yes there is no actual form there. I’m simply using the URL string part where it says id=12345

So when you go to www.mywebsitedomiannamegoeshereitellsya.com/?id=12345 the query is updated as you show in the example:


$id = (int) $_GET['id'];  
UPDATE tbl SET live = '1' WHERE id = $id; 
// if successfull echo yay

So what i’d really like to know is if I simply typecast id to an int like the above example is that ‘safe’ and no attacks can occur?

4

Whether you use POST or GET is not critical. It’s just that I always use POST unless I want the user to be able to bookmark the url in their browser. It’s much more important to validate and sanitise all user inputs in your script before you process them.

To change from GET to POST, “in theory” all you should need to do is change the form’s action attribute to “post” and then do a global replace in your php script and change all the $_GET to $_POST.

5

I forgot to mention that part of the sanitasion should include enclosing all inputs in a sql query in single or double quotes.

I enclose all query inputs in single quotes as well as use mysql_real_escape_string(). Maybe it’s overkill but you can’t be too careful on the www nowadays :shifty:

6

It doesn’t hurt to mysql_real_escape() all user input data just in case your validation code “misses” some sql injection code in any of the GET or POST data.

7

  1. I recommend you use POST and not GET

  2. validate all user inputs in your server side script before you process them in anyway.

  3. use mysql_real_escape_string() to sanitise all user inputs before putting them in an sql query.

  4. or use stored procedures to run your queries.

8

I don’t see any problems with using the $_GET as long as you sanatize the data you GET. Of course, you also need to sanatize any POSTed data as well.

Using the GET method also allows you to link direct to your pages although you might want to tidy up the URL’s with a bit of rewriting. Not so useful on a sign up page but ideal for a product listing page.

9

Use prepared statements.

10

I think the post has gotten off track. 7724 since you are getting that variable via user click, you still have to use get I believe. When a user clicks …/signup.php?id=12345, does the page say ‘Congrats {name/email/user name}, you subscribed’ or do they fill out form?

If it is automatic, on signup.php


$id = (int) $_GET['id']; 
 UPDATE tbl SET live = '1' WHERE id = $id;
// if successfull echo yay

if they have to do a form


$id = (int) $_GET['id'];
//query the database to get user info based off $id
$_POST['id] = $id;
/**
 make a form using method='post' and auto fill 
in the info from above, allow users to edit change, etc. 
Sanitize all fields, if all kosher run
UPDATE tbl SET live = '1' WHERE id = $id;
*/
11

No, that’s it. The result is always a number. Try it :slight_smile:

Edit: of course, they could still pass any number. If you don’t want that, you’ll have to have some way of checking if the right number is being passed.

12

$_GET and $_POST to my knowledge always have string type - so is_int() will always return false on plain is_int($_GET[‘id’]) without prior manipulation of it’s value/type.
I use intval():

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

or if I am in a particularly bad mood:

if(strval(intval($_GET['id']))===$_GET['id']){
/*It naturally takes care of such nuances as integer overflows
(with respect to particular PHP installation so it will not accept
hundred digit number as a legal entry) and nonsense like
submitting 0xff or 000000000000000000000000099 */

//Everything is ok - proceed
$id=intval($_GET['id']);
...
}
else {
//Why you &*###@!
die('Someone will pay for this!!!');
}
13

Here is an example of why mysql_real_escape is not enough:
http://bugs.php.net/bug.php?id=46011&edit=1

Also - check out thread on web application security in my signature - there are resources on DB security as well.

14

  1. I recommend you use POST and not GET

  2. validate all user inputs in your server side script before you process them in anyway.

  3. use mysql_real_escape_string() to sanitise all user inputs before putting them in an sql query.

  4. or use stored procedures to run your queries

Thanks Kalon, but in my example how could I avoid using $_GET and use post in this instance?

15

It doesn’t hurt to mysql_real_escape() all user input data just in case your validation code “misses” some sql injection code in any of the GET or POST data.

It doesn’t hurt, but in case of numeric values it just doesn’t work. If you think you are safe from mysql injections just because you pass all user input through mysql_real_escape_string, then you’re wrong.

16

Change the form method from “GET” in “POST”

17

mysql_real_escape_string only works on strings, that is values that in your query are between single quotes. It does not work on numeric values.

18

Take a look at : http://php.net/manual/en/function.mysql-real-escape-string.php

But you might also want to check that $_GET['id] is a number (integer).

19

If the value has to be numeric, cast it with (int).
If the value has to be a string (in your query you put it between single quotes), then use mysql_real_escape_string.

Or you can use PDO

20

Thanks, i’ve done that already:


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

So is that it? Nobody can attack? Should I use some kind of if else to say what to do is this is not TRUE?