views:

267

answers:

5

I am currently using this process to Sanitize/Filter comment entered by users ->
This one is used to strip slashes... and

 if (get_magic_quotes_gpc()) {
        function stripslashes_deep($value)
        {
            $value = is_array($value) ?
                        array_map('stripslashes_deep', $value) :
                        stripslashes($value);

            return $value;
        }

        $_POST = array_map('stripslashes_deep', $_POST);
        $_GET = array_map('stripslashes_deep', $_GET);
        $_COOKIE = array_map('stripslashes_deep', $_COOKIE);
        $_REQUEST = array_map('stripslashes_deep', $_REQUEST);
    }

Then the comment goes through this function to sanitize the data...

   function my_strip_tags($str) {
             $strs=explode('<',$str);
          $res=$strs[0];
             for($i=1;$i<count($strs);$i++)
             {
           if(!strpos($strs[$i],'>'))
            $res = $res.'&lt;'.$strs[$i];
                 else
                     $res = $res.'<'.$strs[$i];
          }
          return strip_tags($res);   
    }

After this it goes straight into the database using prepared statement..

function add_comment($comment,$type,$update_id,$user_id){
      $query="INSERT INTO comment_updates (updateid,userid,comment) VALUES(?,?,?)";
       if($stmt=$this->conn->prepare($query)) {
       $stmt->bind_param('sss',$update_id,$user_id,$comment);
       $stmt->execute();
        if($this->conn->affected_rows==1){
        $stmt->close();
        return true;
        }
      }
     }

I just wanted to know if this is secure enough or if their are any other better alternatives...Thanks

A: 

Your magic quotes handling is fine, although if you create get parameters with quotes you need to stripslashes the keys too. :)

As for strip tags, you are better off with a real HTML filter library. There are so many twists and turns involved with html that you just should not trust anything you just make once and forget about. People spend time making those HTML filters so use their work to your advantage.

As for "straight into the DB", well in a bound parameters, sure, that's great. You can safely put anything into a bound parameter. In a string with quotes, I hope you are escaping the result.

jmucchiello
Do you have links to any good html filter libraries that you've mentioned?
halocursed
I didn't want to list any because I've only really used htmLawed so I can't really give more than one example. But there are others. Google: "PHP HTML filter library"
jmucchiello
+3  A: 

The most important thing when thinking about storing data to a database is to escape it ; using mysql_real_escape_string, or mysqli_real_escape_string, or PDO::quote, depending on the DB you're using (or other functions for oracle/pg/...)

Another solution would be to use prepared statements (see mysqli::prepare and/or PDO::prepare -- those are not supported by the old mysql_* extension), which will deal with escaping data at your place ;-)


When thinking about HTML output, you have two solutions :

  • accept HTML and use some library like HTMLPurifier to filter/clean it ; it will allow to specify exactly which tags and attributes are allowed, and will give you clean and valid HTML as output.
  • try to remove HTML, like you are doinig -- not always working well (what if you forget some special case ? )
  • escape HTML, with htmlentities or htmlspecialchars : not necessarily looking nice, but the output will look like the input of the user.

I would go with either the first or the last solution ; yours feels more "dangerous" -- but that's only a feeling ^^ (the general idea being "do not reinvent the wheel")

Pascal MARTIN
What if I am using prepared statements to insert data(*post edit)...Should I still use mysql_real_escape_string??
halocursed
No. Prepared statements handle the escaping for you.
ceejayoz
A: 

Escape all characters when puting it in database. When retrieving and displaying make sure to escape html formating such as <sometag> so it displays instead of being treated as code.

Jonas B
+2  A: 

Don't write your own HTML sanitizer. You'll create XSS holes.

If you're going to write your own, at least run the ha.ckers.org xss smoketests against it

Between those tests, and the htmlpurifier comparison of filters, you should be able to get a good idea of just how complicated html sanitization is -- and why you should leave it to the pros.

Frank Farmer
"Don't write your own HTML sanitizer. You'll create XSS holes." According to your statement, there it is impossible to build one without holes, if you use 1 that others have made, that would mean it has holes as well if it was built by a human. So I disagree 100%
jasondavis
I only said *he'd* create holes. I didn't say everyone would.XSS is complicated -- that's why even the biggest sites fall victim to it all the time. If you have to ask "is this secure?", then you're nowhere *near* knowledgeable enough to be writing something so critical to your app's security. HTML sanitization is difficult to do right, even for the most experienced developers.
Frank Farmer
A: 

PHP has little known but powerful built in sanitation functions. I would recommend using them:

Input filtering in PHP

filter_input and filter_var

Tonycore