PHP: Is mysql_real_escape_string sufficient for cleaning user input? [duplicate]

Question

Is mysql_real_escape_string sufficient for cleaning user input in most situations?

I'm thinking mostly in terms of preventing SQL injection but I ultimately want to know if I can trust user data after I apply mysql_real_escape_string or if I should take extra measures to clean the data before I pass it around the application and databases.

I see where cleaning for HTML chars is important but I wouldn't consider it necessary for trusting user input.

Kassandra

XSS is a huge problem, if you ignore it you're asking for problems of equal magnitude. You must validate on the way in, but if you aren't stripping javascript when you display user content it's really, really trivial to do things like Session Hijacking and XSS attacks which is frankly, waaay easier to use than SQL injection but is a little newer so people don't get it yet. – Margret

mysql_real_escape_string is only intended to protect against SQL injections. For other vulnerabilities you need other tools. – Zigrang

@Zigrang mysql_real_we_really_mean_it_this_time_escape_string would cover those cases. When it is added in the near future. – Anglicist

@Anglicist hilarious, but bollocks, no one function can do all cleaning. – Heteromerous

Escaping the string is not safe! Learn about prepared statements for MySQLi. – Resistor

Answer 1

mysql_real_escape_string is not sufficient in all situations but it is definitely very good friend. The better solution is using Prepared Statements

//example from http://php.net/manual/en/pdo.prepared-statements.php

$stmt = $dbh->prepare("INSERT INTO REGISTRY (name, value) VALUES (?, ?)");
$stmt->bindParam(1, $name);
$stmt->bindParam(2, $value);

// insert one row
$name = 'one';
$value = 1;
$stmt->execute();

Also, not to forget HTMLPurifier that can be used to discard any invalid/suspicious characters.

...........

Edit: Based on the comments below, I need to post this link (I should have done before sorry for creating confusion)

mysql_real_escape_string() versus Prepared Statements

Quoting:

mysql_real_escape_string() prone to the same kind of issues affecting addslashes().

Chris Shiflett (Security Expert)

Mclendon

Give an example of where it is not sufficient, but where Prepared Statements are. – Colubrid

@Marius: Especially in situations when characters are encoded with UTF8, hex ,etc. If you research on internet, you would find that most developers suggest using Prepared statements over mysql_real_escape_string if possible. Sorry at this time i can't remember the link. Hope that clarifies, sorry for bad english :( – Mclendon

While I agree that prepared statements are superior, I would also be very interested in knowing when mysql_real_escape_string() is insufficient for escaping data used in a query. – Dogberry

mysql_real_escape_string was designed to take the connection character set into account, and yes, it is safe. addslashes() is not safe in all circumstances. – Greyhound

Actually no, mysql_real_escape_string isn't always safe. you should red my post. – Lorolla

@The Rook: It's not mysql_real_escape_and_quote_string(). Failure to correctly quote (or in your example cast the value as an integer) is not a problem with mysql_real_escape_string(). – Dogberry

hello all, i have updated my answer in an endeavor to do away with the confusion created. plz check thanks. – Mclendon

And do not forget the performance boost you get from using Prepared Statements! – Wirer

Answer 2

The answer to your question is No. mysql_real_escape_string() is not suitable for all user input and mysql_real_escape_string() does not stop all sql injection. addslashes() is another popular function to use in php, and it has the same problem.

vulnerable code:

mysql_query("select * from user where id=".mysql_real_escape_string($_GET[id]));

poc exploit:

http://localhost/sql_test.php?id=1 or sleep(500)

The patch is to use quote marks around id:

mysql_query("select * from user where id='".mysql_real_escape_string($_GET[id])."'");

Really the best approach is to use parametrized queries which a number of people ahve pointed out. Pdo works well, adodb is another popular library for php.

If you do use mysql_real_escape_string is should only be used for sql injection, and nothing else. Vulnerabilities are highly dependent on how the data is being used. One should apply security measures on a function by function basis. And yes, XSS is a VERY SERIOUS PROBLEM. Not filtering for html is a serious mistake that a hacker will use to pw3n you. Please read the xss faq.

Lorolla

What does that have to do with mysql_real_escape_string()'s ability to prevent injection? Properly quote or cast your value as an integer. – Dogberry

@Dogberry You could cast it, you could also forget to cast it. If you use parametrized quires so you KNOW it is always safe, even if you have 15 drunken monkeys working on the project. – Lorolla

Right, but that doesn't make mysql_real_escape_string() unsuitable for escaping strings for SQL. Edit: Yes, prepared statements are better. – Dogberry

@Dogberry no one on this thread has pointed out it was just for strings. Many vulnerabilities have been written by people who have posted on this very thread becuase of this misunderstanding. This is also true for addslashes(). – Lorolla

Right, again. But like I mentioned above it's not mysql_real_escape_and_quote_string(). And again, not inherently a problem with the function but with its incorrect use. I'd still like to see an example where mysql_real_escape_string() is insufficient. – Dogberry

No, clearly I don't. Your argument seems akin to saying while() is not useful for iteration because you can do while(1); – Dogberry

Thank you for pointing this out. In this situation, I like like to wrap a floatval() around anything that is going in as a number. – Croup

@Tobias if you aren't using parameterized queries you have bigger problems on your hands. – Lorolla

Answer 3

To the database, yes. You'll want to consider adequately escaping / encoding data for output as well.

You should also consider validating the input against what you expect it to be.

Have you considered using prepared statements? PHP offers numerous ways to interact with your database. Most of which are better than the mysql_* functions.

PDO, MDB2 and the MySQL Improved should get you started.

Dogberry

"To the database" - so that would be output then. Not input. – Klimesh

Answer 4

What situations?

For SQL queries, it's great. (Prepared statements are better - I vote PDO for this - but the function escapes just fine.) For HTML and the like, it is not the tool for the job - try a generic htmlspecialchars or a more precise tool like HTML Purifier.

To address the edit: The only other layer you could add is data valdation, e.g. confirm that if you are putting an integer into the database, and you are expecting a positive integer, you return an error to the user on attempting to put in a negative integer. As far as data integrity is concerned, mysql_real_escape_string is the best you have for escaping (though, again, prepared statements are a cleaner system that avoids escaping entirely).

Soulsearching

Again, the answer is nearly right - but the question was about input - not output – Klimesh

Sometimes input goes straight to the output, e.g. <?php echo $_GET['name']; ?>. – Soulsearching

Answer 5

mysql_real_escape_string() is useful for preventing SQL injection attacks only. It won't help you with preventing cross site scripting attacks. For that, you should use htmlspecialchars() just before outputting data that was originally collected from user input.

Damato

SQL injection is so easy to understand, XSS is a huge problem right now though. Thank you for bringing this up. – Margret

Answer 6

There are two ways, one is to use prepared statements (as mentioned in other answers), but that will slow down your app, because you now have to send two requests to the Database, instead of one. If you can live with the reduced performance, then go for it; Prepared Statements makes your code prettier and easier to deal with.

If you chose to use mysql_real_escape_string, then make sure that you escape all the strings that are untrusted. An (mysql_real_escape_string) escaped string is SQL Injection secure. If you don't escape all the strings, then you are not secure. You should really combine mysql_real_escape_string with input validation; checking that a variable you expect to hold a number really is a number and within the expected range. Remember, never trust the user.

Colubrid

Answer 7

There are different types of "cleaning".

mysql_real_escape_string is sufficient for database data, but will still be evaluated by the browser upon display if it is HTML.

To remove HTML from user input, you can use strip_tags.

I would suggest you look into using PDO instead of regular MySQL stuff, as it supports prepared statements right out of the box, and those handle the escaping of invalid data for you.

Beau

Answer 8

You can try both, as in

function clean_input($instr) {

     // Note that PHP performs addslashes() on GET/POST data.
     // Avoid double escaping by checking the setting before doing this.
    if(get_magic_quotes_gpc()) {
        $str = stripslashes($instr);
    }
    return mysql_real_escape_string(strip_tags(trim($instr)));
}

Lassiter

I used this technique. Thanks @crafter. – Usufruct

Answer 9

The best way to go would be to use Prepared Statements

Jubbah

2 years ago, tons of other answers in this thread - does it really matter? – Jubbah

Answer 10

I thought I'd add that PHP 5.2+ has input filter functions that can sanitize user input in a variety of ways.

Here's the manual entry as well as a blog post [by Matt Butcher] about why they're great.

Kassandra

be aware that this does not help you against sql-injections, but against XSS! – Contumacy