Is this safe against major XSS attacks?

I am building a classifieds website here in Portugal, and I'm now in the security phase. So until now, I already made what I think is a good measure against SQL injection:

$firstname = chunk_split(mysql_real_escape_string($_POST[firstname]),1,'.');
$lastname = chunk_split(mysql_real_escape_string($_POST[lastname]),1,'.');
$email = chunk_split(mysql_real_escape_string($_POST[email]),1,'.');

etc...

This will save valid and invalid emails, for example, like this:

good email: u.s.e.r.@.e.m.a.i.l.h.o.s.t...c.o.m.
bad email:

Example of text in the email field:

Y';
 UPDATE table
  SET email = 'hacker@ymail.com'
  WHERE email = 'joe@ymail.com';

and after stored in the database:

Y.\.'.;. .U.P.D.A.T.E. .t.a.b.l.e. .S.E.T. .e.m.a.i.l. .=. .\.'.h.a.c.k.e.r.@.y.m.a.i.l...c.o.m.\'. .W.H.E.R.E. .e.m.a.i.l. .=. .\.'.j.o.e.@.y.m.a.i.l...c.o.m.\.'.;.

So, I don't care if the size of the stored string is bigger because I think this is a solid approach and a very fast one. And if I had to use other ways, the scripts would probably consume the same time that is here exchanged by size.

But now I have the serious problem of XSS attacks. I'm trying to prevent only attacks based on text, not images or JavaScript because I will not have untrusted data there.

The solution I find is based on the last example, and looks like this:

$str = "alert('XSS attack');";
$ad_title = chunk_split($str,1,".");

So when the page loads the users will see the inserted text and the alert will not work:

alert('fabio');

This is the source code:

.s.c.r.i.p.t.>.a.l.e.r.t.(.'.f.a.b.i.o.'.).;../.s.c.r.i.p.t.>.

Without being concerned about all the code it produces, my question is: is this enough or actually safe?

Is it safe? Maybe. Is it the right way to do it? No.

Even if it were safe, there’s a big problem with the SQL, and that’s that you’re inserting the dots after escaping, rather than before. I notice that for the input ', it will be escaped to \', and then your dot-insertion turns it into \.'.. Before, the \ was escaping the ', but now it isn’t. Now it’s escaping the .. I don’t know if a malicious entity could do anything bad with that, but I wouldn’t take any chances.

And here are some problems with the HTML:

• Copy-and-pasting will catch those dots.

• Search engines will catch those dots.

• You break any Unicode characters, e.g. you turn 日本語 into ?.?.?.?.?.?.?.?.?.. (But I only have an old version of PHP installed; maybe newer versions are more intelligent, but in that case, you might have other problems like putting a combining character onto your >)

And that’s not to mention the gigantic size increase, and potentially having invalid HTML… all around, it’s just not a very good way to do it.

So what’s the right way to do it? Well, for inserting data into the database, you should be using PDO with prepared statements. Then you don’t have to deal with SQL escaping at all: you prepare a query with placeholders, and send in the placeholder data, and since the data doesn’t have to touch the query, you don’t have to worry about that at all.

Sidebar: using PDO

You said that it’d be a lot of work to use PDO. Well, I’ve never found it particularly difficult. Your code perhaps looks like this:

mysql_connect('localhost', 'myapp', 'letmein');
mysql_select_db('myapp');
// ...
mysql_query("insert into users (firstname, lastname, email) values ('$firstname', '$lastname', '$email')");

But it’s not actually that hard to use PDO. That code could be translated to use prepared statements and PDO like so:

$db = new PDO('mysql:host=localhost;dbname=myapp', 'myapp', 'letmein');
// ...
$stmt = $db->prepare('insert into users (firstname, lastname, email) values (:firstname, :lastname, :email)');
$stmt->bindValue('firstname', $_POST['firstname']);  // look ma, no escaping!
$stmt->bindValue('lastname', $_POST['lastname']);
$stmt->bindValue('email', $_POST['email']);
$stmt->execute();

And besides being more secure, you’re also moving onto something that the PHP developers have committed to keep in place: the mysql_* functions will be removed, as the PHP documentation says:


Warning: This extension is deprecated as of PHP 5.5.0, and will be removed in the future. Instead, the MySQLi or PDO_MySQL extension should be used. See also MySQL: choosing an API guide and related FAQ for more information.

End of sidebar

On the HTML side, using htmlentities or htmlspecialchars is the standard way to do it.

Then, finally, I’d like to point out one thing about how you’re accessing POST data: you’re currently using $_POST[firstname]. It’s supposed to be an expression that’s between the brackets, but you have the name of the field literally in there. It turns out that that’s okay currently, as undefined constants evaluate to their name. But that does generate a notice, and depending on your error reporting level, you will probably see lots of notices in your error log saying that this is bad and/or deprecated. You should explicitly quote it: $_POST['firstname'].

StackExchange Code Review Q#60334, answer score: 18