views:

165

answers:

4

Is this good enough to avoid a SQL injection?

mysql_real_escape_string(htmlentities (urlencode($_POST['postmessage'])));
+19  A: 

mysql_real_escape_string() is the only method you need here.
You shouldn't do an htmlentities() nor urlencode() before inserting data in your database. These methods are usually code during the rendering of the View you offer to your users.

A better way to avoid SQL Injection is the use of prepared statements.


Resources :

On the same topic :

Colin Hebert
+1 for prepared statements. They can help with performance too in the right situations.
Chris Thompson
+1 for prepared statements
Steve Tranby
<3 Prepared Statements :)
Xeross
+1  A: 

You should also make sure to use " arround the place, where you insert your code.

For example if you do

$_POST['userid'] = mysql_real_escape_string($_POST['userid']);
mysql_query('SELET * FROM user WHERE userid = '. $_POST['userid']);

mysql_real_escape_string wount help anything. Its because $_POST['userid'] is not surrounded by '.

So you should do

$_POST['userid'] = mysql_real_escape_string($_POST['userid']);
mysql_query('SELET * FROM user WHERE userid = \''. $_POST['userid'] .'\'');

instead.

So using mysql_real_escape_string on your variables does not automaticly mean they are secure in any query.

Another approach would be to use Prepared statements.

JochenJung
+1  A: 

Yes, but there's a reason for not using mysql_real_escape_string(). First, it's a pain to type. Second, you have to remember to use it every time. Third, it makes your code ugly. Fourth you have to remember to quote your strings. Fifth, it's more difficult to insert blobs in a db this way.

Learning PDO will make your life better in the long run. It's more difficult to learn than simply using mysql_real_escape_string(), but the long term benefits outweigh the inconvenience of a learning curve.

Jason Thompson
+6  A: 

I think that you are confusing two security issues: SQL injection and cross-site scripting (XSS).

A website is vulnerable to SQL injection when improperly sanitized user input is used in an SQL query that is sent to the SQL database. This code, for example, introduces an SQL injection vulnerability:

mysql_query("INSERT INTO postmessages (postmessage) VALUES ('" . $_POST['postmessage'] . "')");

This problem is easy to fix by escaping the user input with a function like mysql_real_escape_string:

mysql_query("INSERT INTO postmessages (postmessage) VALUES ('" . mysql_real_escape_string($_POST['postmessage']) . "')");

That's all that you need to do, but the tricky part is remembering to do this for every piece of user input that is used in an SQL statement.

A website is vulnerable to cross-site scripting when user input is used in HTML that is sent to a client. This code, for example, introduces a XSS vulnerability:

echo "<div class='postmessage'>" . $_POST['postmessage'] . "</div>";

A XSS vulnerability is fixed by escaping the user input with a function like htmlspecialchars:

echo "<div class='postmessage'>" . htmlspecialchars($_POST['postmessage']) . "</div>";

Again, this is easy to do, but easily forgotten.

Usually, user input that is placed in a database to be used in sending back HTML at a later time is saved unmodified. That is, only mysql_real_escape_string is used. However, you could escape user input to prevent XSS, and then escape the XSS-safe string to prevent SQL injection:

mysql_query("INSERT INTO postmessages (postmessage) VALUES ('" . mysql_real_escape_string(htmlspecialchars($_POST['postmessage'])) . "')");

The benefit is that you don't need to remember to escape values from the database with htmlspecialchars before writing them into HTML. The drawback is that some values may need to be escaped with different functions. For example, a user name would probably be escaped with htmlspecialchars, but a "postmessage" might allow BBcode, Markdown, or a subset of HTML. If you escaped all input to prevent XSS, then you would need to unescape values from the database with, for example, htmlspecialchars_decode.

One problem is that unescaping the escaped string does not always return the original string (unescape(escape($orig)) is not necessarily the same as $orig). Even with htmlspecialchars and htmlspecialchars_decode, using a different quote style will cause this problem. Another example is that if strip_tags is used, then information is removed irrecoverably; you will not be able to undo the strip_tags. Thus, many developers choose to use mysql_real_escape_string only to save values into the database and htmlspecialchars (or whatever) to prepare a string from the database to be used in HTML.

Daniel Trebbien
Wouldn't it be better to use prepared statements, cleaner to look at and possibly easier to work with once you get the hang of it.
Xeross
@Xeross: Definitely. Prepared statements make it easier to consistently escape user input. In answering OP's question, however, I wanted to make sure that s/he understood the reasons for using `mysql_real_escape_string` (or some DB escaping mechanism) and `htmlentities` (or some XSS escaping mechanism).
Daniel Trebbien