0

Is this enough?

function cleanVar($str1){
 if(get_magic_quotes_gpc() == 0){
  $str1 = addslashes(htmlspecialchars($str1));
 }
 $str1 = stripslashes($str1);
 $str1 = htmlspecialchars($str1);
 $str1 = strip_tags($str1);
 $str1 = mysql_real_escape_string($str1);
 $str1 = str_replace("script","",$str1);
 $str1= str_replace("body","",$str1);
 $str1 = str_replace("select","",$str1);
 $str1= str_replace("insert","",$str1);
 $str1= str_replace("update","",$str1);
 $str1 = str_replace("on","",$str1);
 $str1= str_replace("<","&l",$str1);
 $str1 = str_replace(">","&",$str1);
 $str1 = trim($str1);
 return $str1;
}
Mat
  • 202,337
  • 40
  • 393
  • 406
webdad
  • 11
  • That's *way* too much. See [The ultimate clean/secure function](http://stackoverflow.com/q/4223980) – Pekka Feb 05 '12 at 16:08
  • 3
    `mysql_real_escape_string` alone is enough. – Gumbo Feb 05 '12 at 16:08
  • @Gumbo and whoever upvoted him - this is false statement. speaking of sanitization, it should be escaping **and quoting** – Your Common Sense Feb 05 '12 at 16:09
  • Is really not necessary to use str_replace() function. – devasia2112 Feb 05 '12 at 16:09
  • What database library are you using? – Pekka Feb 05 '12 at 16:09
  • Security by smashing all incoming data into little bits. Eugh. Protect your data [against SQL injection](http://stackoverflow.com/questions/60174/best-way-to-stop-sql-injection-in-php) and protect your HTML (not your database) [against XSS](http://stackoverflow.com/questions/71328/what-are-the-best-practices-for-avoiding-xss-attacks-in-a-php-site). – Quentin Feb 05 '12 at 16:10
  • @webdad, this is most useless and overkill function I've ever seen. – Your Common Sense Feb 05 '12 at 16:11
  • @Col.Shrapnel Yes, it does depend on the context inside the MySQL statement the value is inserted into. – Gumbo Feb 05 '12 at 16:13

3 Answers3

3

If at all possible, use PDO & prepared statements. It handles it all for you and ensures that you aren't losing any data (there's some weirdness with strip tags)

Troy McCabe
  • 494
  • 3
  • 8
  • 1
    The code in the question appears to be trying to deal with XSS as well as SQL injection. Prepared statements (or rather, bound parameters) won't help against that. – Quentin Feb 05 '12 at 16:12
  • The question was "secure php variable before insert it to database." I made no inference from code about what else needed to be done. – Troy McCabe Feb 05 '12 at 17:16
1
$str1 = str_replace("script","",$str1);
$str1= str_replace("body","",$str1);
$str1 = str_replace("select","",$str1);
$str1= str_replace("insert","",$str1);
$str1= str_replace("update","",$str1);
$str1 = str_replace("on","",$str1);

What will happen is you apply this function to a string containing: "This is a comment on the update on the situation in Iraq". You will strip away way too much information.

PDO is brilliant and you should consider switching from the outdated mysql-library to it.

OptimusCrime
  • 14,662
  • 13
  • 58
  • 96
  • not PDO but parameterized queries it is called. PDO holds no monopoly for them and yet lets you to do it wrong wey. I am also curious, do you use PDO yourself? – Your Common Sense Feb 05 '12 at 16:18
  • Ive used mysql for years, but I recently switched to http://xpdo.org/ , an ORB powered by PDO. – OptimusCrime Feb 05 '12 at 16:19
-1

Looking at the OWASP prevention sheets this pretty much covers is

 & --> &amp;
 < --> &lt;
 > --> &gt;
 " --> &quot;
 ' --> &#x27;     &apos; is not recommended
 / --> &#x2F;     forward slash is included as it helps end an HTML entity

https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet

SQL injection is also included in this.

Also it's kind of weird to replace the actual SQL statements, it can become a problem if you are going to save actual words in your database.

Rohan
  • 8,006
  • 5
  • 24
  • 32