A little confusion about security

When saving data to the DB I use the following method:

Code:

public function strToDB($string) {
   if(is_array($string)) {
       foreach($string as $key => $value) {
           $string[$key] = $this->clean_string($value);
       }
       return $string;
   } else {
       $string = htmlspecialchars($string, ENT_QUOTES, "UTF-8");
       $string = mysql_real_escape_string($string);
       $string = str_replace('\r\n', PHP_EOL, $string);
       $string = str_replace('\n', PHP_EOL, $string);
       return $this->xss_clean($string);
   }
}

This method will convert special chars for saving in your DB

In most cases, you should use $this->db->escape() rather than mysql_real_escape_string(). $this->db->escape() will use the function appropriate for the currently configured database and will be more likely to continue to work in PHP 7, since the mysql_* functions were deprecated in 5.5 and removed in 7. Additionally, escaping the data should be the last thing done before putting the data into the database, which is why many of the database functions do it for you.

Why are you using htmlspecialchars() on data to be stored in a database? The whole purpose of this function is to encode data to be output as HTML. Plus the conversions performed by htmlspecialchars() are limited to a handful of characters, and, by setting ENT_QUOTES, you've ensured that at least two of the characters handled by mysql_real_escape_string() will no longer be in your strings (and nearly half of those which are left you're going to go on to mess with after you call mysql_real_escape_string(), though I don't know how you expect to find a '\r\n' after you've replaced them all with '\\r\\n').

Further, if you use CI's html_escape() instead of htmlspecialchars(), it would at least use config_item('charset') as the third parameter to ensure the encoding matches what's used in the rest of the system (as should be done on html_entity_decode() as well), though this still wouldn't be the appropriate place to use the function.

Finally, to quote the docs for the Input class:

XSS escaping should be performed on output, not input!

Perhaps that should be even more specific, in that it should be performed on output to HTML.

When retrieving data from the DB I use the following method

Code:

public function strFromDB($str) {
    if( is_string($str) ) {
        $str = trim(html_entity_decode($str, ENT_QUOTES, "UTF-8"));
        $str = stripslashes($str);
        return $str;

    } else {
        return $str;
    }
}

The method will reverse what the first method did, so that you can edit the data or print to screen.

No, this does not reverse what the first method did. html_entity_decode() does the opposite of htmlentities(), which could be quite a bit more than just reversing html_special_chars(). Further, between this and stripslashes, you've potentially undone some of the work done by xss_clean(), and this is the point at which it might be appropriate to actually use xss_clean(). (For example, xss_clean() will replace '<?' with '&lt;?' and '?>' with '?&gt;', and html_entity_decode() will convert them back.)

Bonfire

Practical CodeIgniter 3
CodeIgniter Testing Guide