8

I've heard people say (in relation to C#/SQL Server, but also in relation to PHP/MySql): Don't escape strings manually - use stored procedures instead.

Ok, I can accept this suggestion, but why? Many say (including on SO) mysql_real_escape_string() is quite enough, mysql_real_escape_string() is good, mysql_real_escape_string() is the first way of protection.

Why? Is there a case where mysql_real_escape_string() can fail? At least one... I don't need many :)

Cœur
  • 37,241
  • 25
  • 195
  • 267
markzzz
  • 47,390
  • 120
  • 299
  • 507
  • 8
    It's biggest weakness is that _you_ can forget to use it! – Michael Berkowski Dec 19 '11 at 11:50
  • You'll find less usage of stored procedures in MySQL than in SQL Server, but the use of prepared statements is usually what's encouraged with PHP/MySQL APIs. – Michael Berkowski Dec 19 '11 at 11:51
  • 1
    I have never had any problem using mysql_real_escape_string(). I think people say to use stores procedures instead is because there is a chance to forget to use mysql_real_escape_String(). – Vagelis Ouranos Dec 19 '11 at 11:52
  • @Tomalak Geret'kal : what's wrong? – markzzz Dec 19 '11 at 11:57
  • @markzzz: If you don't already feel it, then I can't explain it to you. – Lightness Races in Orbit Dec 19 '11 at 11:58
  • @Tomalak Geret'kal : you are strong! – markzzz Dec 19 '11 at 11:59
  • 2
    @Khronos: "I have never had a problem" does not count for anything. For most people (basic usage, no attacks made against your app), if "they have had a problem" then the whole world would also have had the same problem. Which means that the problem would have been fixed long before you learned what `mysql_real_escape_string` is. – Jon Dec 19 '11 at 12:03
  • See http://stackoverflow.com/questions/110575/do-htmlspecialchars-and-mysql-real-escape-string-keep-my-php-code-safe-from-inje/110576#110576 – Cheekysoft Dec 19 '11 at 12:53
  • possible duplicate of [Why mysql_escape_string is highly discouraged?](http://stackoverflow.com/questions/8233294/why-mysql-escape-string-is-highly-discouraged) – Your Common Sense Dec 20 '11 at 20:11

5 Answers5

9

When mysql_real_escape_string FAIL:

$sql = "SELECT * FROM users WHERE id=" + mysql_real_escape_string($_GET['id']);

If $_GET['user_id'] is set to 1 OR 1=1, there are no special chars and it's not filtered.

The result: All rows are returned.

It gets worse. How about this... what if $_GET['user_id'] is set to 1 OR is_admin = 1?

The function is only designed to be used when inside single quotes.

Cheekysoft
  • 35,194
  • 20
  • 73
  • 86
Moz Morris
  • 6,681
  • 2
  • 20
  • 18
  • I see this so often in the wild – Cheekysoft Dec 19 '11 at 12:49
  • Yeah! Usually if I aspect an int, I check before if the $_GET['user_id'] is int! Yeah, of course I should use PDO for avoid these checks... – markzzz Dec 19 '11 at 13:23
  • 2
    This is more of a misuse of the function than anything else. After all, it is named `mysql_real_escape_string`, not `mysql_real_escape_integer` . It's not mean to be used with integer fields. – NullUserException Oct 09 '12 at 16:28
3

There are two things that can go wrong with mysql_real_escape_string:

  • You can forget to use it
  • If you are using some specific multibyte connection encodings, and you have set these encodings with SET NAMES instead of mysql_set_charset as is proper, it can still leave you vulnerable

Update:

  • You are safe with UTF-8 (although this does not mean that you should continue using SET NAMES!)
  • For an explanation, see here
Jon
  • 428,835
  • 81
  • 738
  • 806
1

Just for info: mysql_real_escape_string() does not escape % and _. These are wildcards in MySQL if combined with LIKE, GRANT, or REVOKE.

Chetan Gawai
  • 2,361
  • 1
  • 25
  • 36
Sudhir Bastakoti
  • 99,167
  • 15
  • 158
  • 162
0

mysql_real_escape_string() can fail to clean the input. Since mysql_real_esacpe_string() takes character set into account while cleaning strings. There's the problem. You can change character via mysql_query function sending the query to change connection's character set. However, mysql_real_escape_string() is oblivious to the set you're using and it will escape some characters improperly.

The other thing is constantly invoking it manually. Even wrapping it in a function is a P.I.T.A. because it implies you have to create some sort of database wrapper / database abstraction layer of your own in order to be able to automate calls to mysql_real_escape_string().

That's why we have PDO in PHP which helps us alleviate all of the above and you get to use parametrized prepared statements which are preferable way of dealing with repeating queries that alter the data.

Prepare the statement, bind input variables and it will clean the input according to the database driver being used and connection's character set. It's less code and completely safe.

N.B.
  • 13,688
  • 3
  • 45
  • 55
  • Nothing is completely safe, even PDO. I.e. it has no mechanism for using table and column names as parameters so you will either way end up using manual sanitation to prevent injections, even with PDO. –  Dec 19 '11 at 12:13
  • @holodoc - if you find yourself in need to bind column and table names via PDO that implies you're receiving column/table names from users' input, which further implies that the way you're doing it is simply wrong. Otherwise, if you have coded your col./tables in an array of your DBAL, then what's the need for additional checking whether they cause an sql error or not? PDO is perfectly safe for saving users' input. mysql_real_escape has a few flaws and they have been pointed out. – N.B. Dec 19 '11 at 13:27
  • Why would using dynamic column and table names imply that those are retrieved from user input? There are far to many cases to name where table/column names are determined by simple flow control. –  Dec 20 '11 at 00:31
  • @holodoc then your argument about PDO being unsafe is moot, why would you use PDO to clean the input (columns, table names) if you're not receiving it from user but from a safe source? Your entire point makes no sense, sorry. – N.B. Dec 20 '11 at 09:08
0

There is only one thing about mysql_real_escape_string() and SQL Injection -

The former does not have a slightest relation to the latter.

Although it sounds paradoxical, nothing can be more true.

Here are 2 statements proving it

  1. You have to escape quoted strings only, as this function helps nothing else.
  2. In fact, you have to escape every string you are adding to the query, even safiest one. just because it may contain some special character and thus break the query (just as an accident, not malicious plot).

Thus, when applicable, this function have to be used anyway, despite of whatever dangers or fears. And in any other case it will help nothing.

The only thing I have to add is that prepared statements do not provide full protection either. Here is the explanation and recommendations: https://stackoverflow.com/a/8255054/285587

Community
  • 1
  • 1
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345