8

Possible Duplicate:
Best way to stop SQL Injection in PHP

If I were to use the $_GET function to retrieve a variable from the URL how can I make it hack proof? Right now I just have addSlashes, what else should I add?

$variable1 = addslashes($_GET['variable1']);
//www.xxxxx.com/GetTest.php?variable1=xxxx
Community
  • 1
  • 1
Albert Renshaw
  • 17,282
  • 18
  • 107
  • 195
  • For future reference this is my function to protect numeric values in the get function (remove the floatval(); function for non numeric get values): `addslashes(mysql_real_escape_string(strip_tags(floatval())));` – Albert Renshaw Dec 30 '11 at 21:23

4 Answers4

21

The first and foremost rule with ANY input, not just $_GET but even with $_POST, $_FILES and anything you read from disk or from a stream you should always VALIDATE.

Now to answer your question in more details, you have several HACKS that exist in this world. Let me show you some:

XSS injections

If you accept data from the URL such as from the $_GET and output this data without stripping out possible tags, you might render your site prone to XSS injection or code injection. For example:

http://myhoturl.com/?search=<script>window.location.href="http://thisisahack.com/"</script>

This would output a hack to your site and people would be redirected to another page. This page could be a phishing attempt to steal credentials

SQL Injection

It is possible to inject SQL to your application. For example:

http://myhoturl.com/?search=%'; UPDATE users SET password=MD5('hello'); SELECT * FROM users WHERE username LIKE '%

Would make your SQL look like this:

SELECT * FROM articles WHERE title LIKE '%%'; UPDATE users SET password=MD5('hello'); SELECT * FROM users WHERE username LIKE '%%';

And thus you'd update all your user's password to Hello and then return something that doesn't match.

This is only a brief overview of what you can do with SQL injection. To protect yourself, use mysql_real_escape_string or PDO or any good DB abstraction layer.

Code injection

Lots of people like to include data from somewhere on the disk and allow uploads of files. For example:

//File igotuploaded.txt
<?php echo 'helloworld'; ?>

And the url allows you to INCLUDE a file by name. ?show=myhotfile.txt

//In this file we include myhotfile.txt
include($_GET['show']);

The person changes that to ?show=../uploads/igotuploaded.txt and you will run echo 'Hello world';

That is dangerous.

rule of thumb... NEVER TRUST USER INPUT, always validate, prevent, validate, fix, validate and again correct...

Good luck

Mathieu Dumoulin
  • 12,126
  • 7
  • 43
  • 71
5

That totally depends on what you are going to do with it:

Without knowing what you are going to do with your data, it is impossible to say what would make it safe.

jeroen
  • 91,079
  • 21
  • 114
  • 132
  • Let's say I was using it the way the person below stated it (I don't currently have a specific use I'm just trying to learn so when I do decide to use get in the future I will use it safely... Anyways, let's say: mysql_query("SELECT userid FROM user WHERE password='".$_GET["variable1"]."';"); – Albert Renshaw Dec 30 '11 at 20:36
  • Passwords you should generally don't read from GET-Variables. Use a password field in POST Form (HTML) instead. And passwords should always be md5()-secured. (For a password you would only need `mysql_real_escape_string();` because you won't print it out. – Dion Dec 30 '11 at 20:41
  • Okay great! Now what about something like viewing a user's profile page and it wasn't a password just an number... like 54038. And I was retrieving and displaying all table entries where that user's number was in a field called "userID" What type of injection could be harmful there and how could I prevent it? – Albert Renshaw Dec 30 '11 at 20:45
  • 1
    @Albert Renshaw You have to know your data, if a field is supposed to be an integer, you'd best cast it to an integer: `(int) $_GET['...']`. – jeroen Dec 30 '11 at 20:48
  • That's great advice! Is there anything to make it AlphaNumeric values only? – Albert Renshaw Dec 30 '11 at 20:55
  • So something like
    hello
    would just show up as "bcenterhellocenterb" Also, I think I recall something about stripping off just html tags so it will just show up as "hello", how do I do that?     – Albert Renshaw Dec 30 '11 at 20:56
  • @Albert Renshaw You'd probably have to resort to regular expressions like `[\w\d]+` but if you get stuck there, you'd better ask a more specific question on SO. – jeroen Dec 30 '11 at 20:57
  • 1
    @Albert Renshaw `strip_tags($string)` – jeroen Dec 30 '11 at 20:59
  • For proper password security, be sure to read [You're Probably Storing Passwords Incorrectly](http://www.codinghorror.com/blog/2007/09/youre-probably-storing-passwords-incorrectly.html). – Bill Karwin Dec 30 '11 at 21:28
  • For filtering in PHP, check out http://php.net/filter – Bill Karwin Dec 30 '11 at 21:28
3

The two greatest risks you face when using user input (any HTTP request counts as user input) are:

You should get familiar with the risks and the defenses. The defenses for each of these threats are different. Using addslashes() is not a complete defense.

A great resource for learning more about secure web programming is the OWASP Top Ten project.

I've done a presentation about SQL Injection Myths and Fallacies that I hope is helpful for you.

Community
  • 1
  • 1
Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
-1

Reading $_GET variables raw isn't dangerous,

The danger usually lies within SQL Injections,

for example:

$_GET["variable1"] = "' OR 1=1 --";

With the query:

mysql_query("SELECT userid FROM user WHERE password='".$_GET["variable1"]."';");

To prevent this:

$safe_input = mysql_real_escape_string($GET["variable1"]);
mysql_query("SELECT userid FROM user WHERE password='".$safe_input."';");
Luc Laverdure
  • 1,398
  • 2
  • 19
  • 36
  • 2
    -1. `mysql_real_escape_string()` doesn't _always_ prevent injections. This might give OP the idea that if he just escapes input, he won't ever have to worry about SQL injections. – kba Dec 30 '11 at 20:32
  • mysql_real_escape_string is good! Also, what other ways are common means of prevention Kristian Antonsen? – Albert Renshaw Dec 30 '11 at 20:36
  • 1
    @AlbertRenshaw `mysql_real_escape_string()` only escapes ticks, quotation marks and the like. Assuming there are none, it isn't helpful at all. An example is with `LIMIT 10, $page` for pagination. If `$page` contains `1; DROP TABLE tbl --`, escaping wouldn't help a bit. Always validate your input and make sure you have the right value (e.g. `is_numeric` and `intval`). – kba Dec 30 '11 at 20:46
  • What is the intuition behind "' OR 1=1 --"? I've seen this statement (or one similar) written before on hack prevention articles.. It makes an error via logic right? Does this intern display an error message with sensitive info? – Albert Renshaw Dec 30 '11 at 20:49
  • is_numeric! Wow! That is great! Ahhh, that is nice! Thankyou! That will help a lot with thing like page numbers! Sweet! Thankyou! +1 – Albert Renshaw Dec 30 '11 at 20:50
  • 2
    @AlbertRenshaw No, it makes no error it just returns true. Consider a bad login system, `SELECT username FROM tbl WHERE username='$user' AND password='$pass'`. If $pass is `' OR 1=1 --`, the query would be `SELECT username FROM tbl WHERE username='$user' AND password='' OR 1=1 --'`, thus return the username and lead the application to believe the correct password was entered. – kba Dec 30 '11 at 20:53
  • Ohhhhh!!! That makes much more sense! Okay, thats good advice, so mysql_real_escape_string(); will prevent this type of hacking? – Albert Renshaw Dec 30 '11 at 21:02
  • Also, for future reference of other clueless people like me. (found this with google) if you are trying to use the function Kristian gave, "intval();" and you have a number but it isn't an integer (e.g 4.389) intval will return 4, but if you want to keep the full numeric value but still strip away things like letters and SQL queries you can use "floatval();" – Albert Renshaw Dec 30 '11 at 21:22
  • No, all `mysql_real_escape_string()` does is that it replaces ' with \', etc., so input cannot break encapsulation. `'foo \' bar'` won't cause an error, but `'foo ' bar'` will. And yes, `intval` only returns 4 - obviously it returns the INTeger value, not the floating point value. :) – kba Dec 30 '11 at 21:55
  • I didn't take the time to make this post elaborate, but seeing all the comments, yes, mysql_real_escape_string() will prevent all SQL injections within a string only, for numeric values, you should verify with is_numeric() before executing mysql_query(). – Luc Laverdure Jan 03 '12 at 15:33