17

I am tring to make my PHP as secure as possible, and the two main things I am trying to avoid are

  • mySQL Injections
  • Cross-Side Scripting (XSS)

This is the script I got against mySQL Injections:

function make_safe($variable) {
$variable = mysql_real_escape_string(trim($variable)); 
return $variable;  }

http://www.addedbytes.com/writing-secure-php/writing-secure-php-1/

Against XSS, I found this:

$username = strip_tags($_POST['username']);

Now I want to unite the two into a single function. Would this be the best way to do so? :

function make_safe($variable) {
$variable = strip_tags(mysql_real_escape_string(trim($variable)));
return $variable; }

Or does the mysql_real_escape_string already prevent XSS? And lastly, is there anything else that I could add into this function to prevent other forms of hacking?

LonelyWebCrawler
  • 2,866
  • 4
  • 37
  • 57
  • 3
    There is no magic wand, just constant vigilance. – Mark Tomlin Jul 28 '11 at 10:56
  • 3
    You and [PDO](http://php.net/manual/en/book.pdo.php) should become friends! – Jacob Jul 28 '11 at 10:59
  • 1
    I don't really understand what the two of you are saying ;) – LonelyWebCrawler Jul 28 '11 at 11:00
  • You should find some useful info in the answers to [this SO question](http://stackoverflow.com/questions/568995/best-way-to-defend-against-mysql-injection-and-cross-site-scripting) and [this question](http://stackoverflow.com/questions/129677/whats-the-best-method-for-sanitizing-user-input-with-php). – ChrisA Jul 28 '11 at 11:07

3 Answers3

14

mysql_real_escape_string() doesn't prevent XSS. It will only make impossible to do SQL injections.

To fight XSS, you need to use htmlspecialchars() or strip_tags(). 1st will convert special chars like < to &lt; that will show up as <, but won't be executed. 2nd just strip all tags out.

I don't recommend to make special function to do it or even make one function to do it all, but your given example would work. I assume.

daGrevis
  • 21,014
  • 37
  • 100
  • 139
14

This function:

function make_safe($variable) 
{
   $variable = strip_tags(mysql_real_escape_string(trim($variable)));
   return $variable; 
}

Will not work

SQL injection and XSS are two different beasts. Because they each require different escaping you need to use each escape function strip_tags and mysql_real_escape_string separatly.
Joining them up will defeat the security of each.

Use the standard mysql_real_escape_string() when inputting data into the database.
Use strip_tags() when querying stuff out of the database before outputting them to the screen.

Why combining the two function is dangerous
From the horses mouth: http://php.net/manual/en/function.strip-tags.php

Because strip_tags() does not actually validate the HTML, partial or broken tags can result in the removal of more text/data than expected.

So by inputting malformed html into a database field a smart attacker can use your naive implementation to defeat mysql_real_escape_string() in your combo.

Johan
  • 74,508
  • 24
  • 191
  • 319
  • So I use mysql_real_escape_string() when adding data to the database, but not when comparing user input with database entries (like when loggin in)? – LonelyWebCrawler Jul 28 '11 at 11:34
  • @user, You use `mysql_real_escape_string()` **always**, on every query, be it select, update delete or insert. – Johan Jul 28 '11 at 15:00
2

What you should really be looking into is using prepared statements and PDO to both provide an abstraction layer against your database as well as completely eradicate SQL injection attacks.

As for XSS, just make sure to never trust user input. Either run strip_tags or htmlentities when you store the data, or when you output it (not both as this will mess with your output), and you'll be all right.

Jon Gjengset
  • 4,078
  • 3
  • 30
  • 43