tags:

views:

79

answers:

5

Sample code:

$email = "" . $_POST['email'];
$con = mysql_connect("localhost","user","pass")
  or die('Could not connect to database.');
mysql_select_db("face", $con);

// Sanitization step
$sanitemail = mysql_real_escape_string($email);

// Is this safe?
mysql_query("INSERT INTO landing_oct_2010 (email) VALUES ('$sanitemail');");

I'd like to know if, for this simple task, whether just using mysql_real_escape_string is fully sufficient to prevent at least injection style SQL attacks, or if there's some other precaution I should take.

The fact that I'm collecting email addresses in this sample is incidental. If I know I'm working with email addresses, I would just throw in a regex and some DNS checks and there I'd have built in validation as well. However, I'd like to focus on the general problem at hand: is the single sanitation function enough?

A: 

Yes it is enough. Compared to addslashes(), mysql_real_escape_string() knows what characters affects MySQL commands. Besides, you can just retrieve and use the data from the database without using stripslashes().

Ruel
You should never need to stripslashes fetched data anyway, unless you somehow ran addslashes on your data twice before you inserted it.
Bill Karwin
as a matter of fact, mysql_real_escape_string() never knows "what characters affects MySQL commands" (whatever you mean), until you tell it to mysql manually
Col. Shrapnel
`mysql_real_escape_string` uses the settings of the last (and often only) connection opened if you don't specify a link ID. It knows, unless you're using two different connections with two different character sets (which is hardly ever the case...and if it were, passing the link ID would fix that).
cHao
A: 

I would use trim to remove spaces also

$sanitemail = mysql_real_escape_string(trim($email));
Drewdin
+3  A: 

Yes, it is sufficient. Well, sort of.

When used properly and consistently, mysql_real_escape_string should be sufficient to prevent SQL injection. That function (unlike mysql_escape_string) takes the character set of your connection into account, so it should work for any site, as long as you actually communicate using the correct character set.

However, in the past, bugs have existed in the escaping routine, which could potentially cause SQL injection to be possible anyway (given the right conditions).

A better choice is to use parameterized queries with the MySQLi library, since that makes escaping completely unnecessary. Parametereized queries basically work by separating the data from the query structure, which in turn means that it is impossible to do SQL injection. Implementation-wise, it's also much harder for the database developers to have introduced a bug that allows SQL injection anyway, again because the query structure is given separately.

Michael Madsen
A: 

As far as I know,there is the something usefull:

    class mysql_db{
        var $connid;

        var $search = array('/union/i', '/load_file(\s*(\/\*.*\*\/)?\s*)+\(/i','/into(\s*(\/\*.*\*\/)?\s*)+outfile/i');
        var $replace = array('union  ', 'load_file   (', 'into   outfile');

        function escape($string){
          if(!is_array($string)) return str_replace(array('\n', '\r'), array(chr(10), chr(13)),mysql_real_escape_string(preg_replace($this->search, $this->replace, $string), $this->connid));
          foreach($string as $key=>$val) $string[$key] = $this->escape($val);
          return $string;
        }
}
T_t
...So the solution is to not allow the word 'union' anywhere in the database without messing it up? Umm, no. The solution is to escape data properly, not mangle it into uselessness.
cHao
the solution is to prevent XSS and other attacks. The argument of function escape is the data which come from a user form or something other from a user.You don't know that what the users will put in a form,so it should be helpful.
T_t
How does it prevent XSS? And what XSS does with database inserts safety?
Col. Shrapnel
This function doesn't prevent *any* kind of XSS, unless your DB is somehow allowed to load data directly from remote URLs (in which case your admin/your DBA/you should be fired immediately). You don't do any kind of HTML-escaping, which is one of the few things that'll always thwart XSS if done properly and consistently (and never undone). All your code does is attempt to prevent a couple of allegedly-dangerous types of queries from being done via SQL injection -- by mangling the data, even! -- and escaping the string properly prevents SQL injection from becoming an issue in the first place.
cHao
A: 

Yes, in general.

But one have to keep in mind some important conditions:

  1. mysql_real_escape_string() is not a "sanitization" function. It doesn't "sanitize" data to make it "safe", but merely escape delimiters. Thus, delimiters are very important part. This function works only as long as processed data being enclosed in quotes. Otherwise it will help nothing.
  2. To put charset magic to work, mysql_set_charset() function should be called first. Otherwise in case of using multibyte encoding, other than utf-8, this function won't help.
Col. Shrapnel