0

I have a php file which at the start, assigns some variables from what was sent using $_GET.

It then does some mysql queries, processes the output, then echos out some text and variables.

The only protection I have set in the code is mysql_real_escape_string on the GETs.

Is that enough to prevent attacks?

What else can be done?

Toby Allen
  • 10,997
  • 11
  • 73
  • 124
David19801
  • 11,214
  • 25
  • 84
  • 127

5 Answers5

5

Well, you take mysql_real_escape_string awfully wrong.
It's not your fault though - its one of wickedest delusions among PHP society. Even official man page put it all wrong.

This function has nothing to do with securing anything in general and GET variables in particular

This function is merely escaping string delimiters, to make string delimiters unable to break a string. Thus, 2 most important consequences:

  • not only GET variables but ALL variables being put into query in quotes should be processed with mysql_real_escape_string(), no matter of their source or origin or possible dangerousness
  • it will have effect on the quoted strings only. It's totally useless to use this function for any other part of query, LIMIT clause variables for example.

Thus, to secure your SQL query, you have to follow whole set of rules, not just deceiving "sanitize your data with mysql_real_escape_string".

You can learn how to protect your SQL from my earlier answer on the similar topic: In PHP when submitting strings to the database should I take care of illegal characters using htmlspecialchars() or use a regular expression?

update
a scenario to show why mysql_real_escape_string is not a silver bullet

being given with url
http://www.example.com/news.php?offset=99999+UNION+SELECT+password+FROM+users+--

a code

$offset = mysql_real_escape_string($_GET['offset']);
$sql    = "SELECT title FROM news LIMIT $offset,20";

Will result if not in not so pompous as little bobby tables' one but in somewhat no less disastrous.

Community
  • 1
  • 1
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • Just to clarify it a bit, what Shrapnel says is not a reason to not use `mysql_real_escape_string`. You should still use it, but you need to be careful on every other part of the site to make sure there are no other vulnerabilities! – gnur Mar 02 '11 at 09:48
  • @gnur where did I say not to use it? **ALL variables being put into query in quotes should be processed with mysql_real_escape_string()** - is what you get as `to not use`? Is my English THAT poor? – Your Common Sense Mar 02 '11 at 09:52
  • No you didn't, but your bold text implied that it had no use, just thought to add a comment to make it absolutely clear that you should use it. – gnur Mar 02 '11 at 11:02
  • @gnur yeah, now I see, thank you. I'll think how to improve it – Your Common Sense Mar 02 '11 at 11:03
0

No, there are plenty of attacks that you might not have protection for. One example is CSRF. It's a big field, so I recommend reading up on this stuff on the Owasp site:

http://www.owasp.org/

Evert
  • 93,428
  • 18
  • 118
  • 189
0

Using this is definitely not sufficient. It is not even sufficient when you only consider sql injection. It is sufficient when you consider sql injection on strings only, but as soon as you have an integer (say an id) it goes wrong:

http://example.com/foo.php?id=10

Goes through:

$q = "SELECT * FROM foo where id = " + mysql_real_escape_string($_GET['id'])

Which results in de SQL query:

SELECT * FROM foo where id = 10

This is easily exploitable, for instance:

http://example.com/foo.php?id=10%3B%20DROP%20TABLE%20foo

Goes through:

$q = "SELECT * FROM foo where id = " + mysql_real_escape_string($_GET['id'])

Which results in de SQL query:

SELECT * FROM foo where id = 10;DROP TABLE foo

I hope this clarifies why it isn't enough.

How you should solve this? Define what input is allowed, and check that the input is indeed of that form, for instance:

if(preg.match("^[0-9]+$",$_GET['id']){
  // your code here
}else{
  // invalid id, throw error
}

But the best way to be on the safe side (regarding SQL Injection) is using prepared statements: http://php.net/manual/en/pdo.prepared-statements.php

markijbema
  • 3,985
  • 20
  • 32
0

mysql_real_escape_string will only protect you agains SQL Injections when you use the return value in a MySQL string declaration like:

'SELECT foo FROM bar WHERE quux="'.mysql_real_escape_string($val).'"'

It won’t protect you if you use it in any other context (specifying the sorting order with ASC/DESC, row limits, table/row names, etc.). In that case you’ll need to validate/filter/sanitize the value separately.

And if the user data can also be part of the query result that you are about to output, use htmlspecialchars to replace any HTML special character.

Gumbo
  • 643,351
  • 109
  • 780
  • 844
-1

you have if the get variables have values using the isset() and empty() functions

Muhamad Bhaa Asfour
  • 1,075
  • 3
  • 22
  • 39